Enable Batch Sorted Merge for nullable order by with firstlast index

Author: natalya-aksman

tsl/src/nodes/columnar_scan/columnar_scan.c

@@ -2844,6 +2844,8 @@ is_var_notnull(const CompressionInfo *compression_info, Var *var)
 #if PG17_LT
 	notnull = ts_get_attnotnull(compression_info->chunk_rte->relid, var->varattno);
 #else
+	/* Since PG18 "notnullattnums" contain only NOT NULL columns with validated NOT NULL constraints
+	 */
 	notnull = bms_is_member(var->varattno, compression_info->chunk_rel->notnullattnums);
 #endif
 
@@ -2895,8 +2897,8 @@ is_var_notnull(const CompressionInfo *compression_info, Var *var)
 static bool
 match_pathkeys_to_compression_orderby(List *pathkeys, List *chunk_em_exprs,
 									  int starting_pathkey_offset,
-									  const CompressionInfo *compression_info, bool for_bsm,
-									  bool *out_reverse)
+									  const CompressionInfo *compression_info,
+									  bool for_batch_sorted_merge, bool *out_reverse)
 {
 	int compressed_pk_index = 0;
 	for (int i = starting_pathkey_offset; i < list_length(pathkeys); i++)
@@ -2925,13 +2927,13 @@ match_pathkeys_to_compression_orderby(List *pathkeys, List *chunk_em_exprs,
 			return false;
 		}
 
-		if (for_bsm)
+		/* Special handling for Batch Sorted Merge with minmax-only index */
+		if (for_batch_sorted_merge &&
+			orderby_sparse_kind(compression_info->settings, orderby_index) !=
+				ORDERBY_SPARSE_FIRSTLAST)
 		{
-			/* Bail out on Batch Sorted Merge if orderby column is nullable,
-			 * as at the moment the minmax metadata we have doesn't include NULLs,
-			 * so it's difficult to use it for null-sensitive ordering.
-			 * But this restriction can be lifted in the future on new type of chunks
-			 * with NULL-handling metadata.
+			/* Bail out on Batch Sorted Merge if orderby column is nullable
+			 * and does not have firstlast index
 			 */
 			if (!is_var_notnull(compression_info, var))
 			{
@@ -2946,9 +2948,7 @@ match_pathkeys_to_compression_orderby(List *pathkeys, List *chunk_em_exprs,
 			 * will be sorted before  [(1,1) ..  (1,19)] with min(1),(1)
 			 * but it should be sorted after as (1,20) > (1,1): correct with firstlast index.
 			 */
-			if (compressed_pk_index > 1 &&
-				orderby_sparse_kind(compression_info->settings, orderby_index) !=
-					ORDERBY_SPARSE_FIRSTLAST)
+			if (compressed_pk_index > 1)
 			{
 				return false;
 			}
@@ -3167,7 +3167,7 @@ build_sortinfo(PlannerInfo *root, const Chunk *chunk, RelOptInfo *chunk_rel,
 														  chunk_em_exprs,
 														  /* starting_pathkey_offset = */ 0,
 														  compression_info,
-														  /* for_bsm = */ true,
+														  /* for_batch_sorted_merge = */ true,
 														  &sort_info.reverse);
 			}
 			return sort_info;
@@ -3199,7 +3199,7 @@ build_sortinfo(PlannerInfo *root, const Chunk *chunk, RelOptInfo *chunk_rel,
 													  chunk_em_exprs,
 													  /* starting_pathkey_offset = */ 0,
 													  compression_info,
-													  /* for_bsm = */ true,
+													  /* for_batch_sorted_merge = */ true,
 													  &sort_info.reverse);
 		}
 		return sort_info;
@@ -3215,12 +3215,13 @@ build_sortinfo(PlannerInfo *root, const Chunk *chunk, RelOptInfo *chunk_rel,
 	 * loop over the rest of pathkeys
 	 * this needs to exactly match the configured compress_orderby
 	 */
-	sort_info.use_compressed_sort = match_pathkeys_to_compression_orderby(pathkeys,
-																		  chunk_em_exprs,
-																		  i,
-																		  compression_info,
-																		  /* for_bsm = */ false,
-																		  &sort_info.reverse);
+	sort_info.use_compressed_sort =
+		match_pathkeys_to_compression_orderby(pathkeys,
+											  chunk_em_exprs,
+											  i,
+											  compression_info,
+											  /* for_batch_sorted_merge = */ false,
+											  &sort_info.reverse);
 
 	return sort_info;
 }

tsl/test/expected/compression_sorted_merge.out

@@ -941,8 +941,8 @@ SELECT 1 as one, 2 as two, 3 as three, x2, x1, c2, time FROM test1 ORDER BY time
    1 |   2 |     3 |  3 |  1 | 43 | Fri Dec 31 17:00:00 1999 PST
    1 |   2 |     3 |  2 |  1 | 43 | Fri Dec 31 16:00:00 1999 PST
 
--- Test with null values: should not optimize
-set timescaledb.debug_require_batch_sorted_merge to 'forbid';
+-- Test with null values: should optimize with firstlast index
+set timescaledb.debug_require_batch_sorted_merge to 'force';
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 ASC NULLS FIRST;
              time             | x2 
 ------------------------------+----
@@ -959,6 +959,8 @@ SELECT time, x2 FROM test_with_defined_null ORDER BY x2 DESC NULLS LAST;
  Sat Jan 01 00:00:00 2000 PST |   
  Sat Jan 01 00:00:00 2000 PST |   
 
+-- should not optimize (NULL order wrong)
+set timescaledb.debug_require_batch_sorted_merge to 'forbid';
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 ASC NULLS LAST;
              time             | x2 
 ------------------------------+----

tsl/test/expected/compression_sorted_merge_unordered.out

@@ -1,6 +1,7 @@
 -- This file and its contents are licensed under the Timescale License.
 -- Please see the included NOTICE for copyright information and
 -- LICENSE-TIMESCALE for a copy of the license.
+\c :TEST_DBNAME :ROLE_SUPERUSER
 -- Increase the working memory limit slightly, otherwise the batch sorted merge
 -- will be penalized for segmentby cardinalities larger than 100, where it is
 -- still faster than sort.
@@ -1295,8 +1296,8 @@ SELECT 1 as one, 2 as two, 3 as three, x2, x1, c2, time FROM test1 ORDER BY time
    1 |   2 |     3 |  2 |  1 | 43 | Fri Dec 31 16:00:00 1999 PST
    1 |   2 |     3 |  2 |  1 | 43 | Fri Dec 31 16:00:00 1999 PST
 
--- Test with null values in x2
-set timescaledb.debug_require_batch_sorted_merge to 'forbid';
+-- Test with null values in x2: batch sorted merge supported with firstlast indexes
+set timescaledb.debug_require_batch_sorted_merge to 'force';
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 ASC NULLS FIRST;
              time             | x2 
 ------------------------------+----
@@ -1329,6 +1330,8 @@ SELECT time, x2 FROM test_with_defined_null ORDER BY x2 DESC NULLS LAST;
  Sat Jan 01 00:00:00 2000 PST |   
  Sat Jan 01 00:00:00 2000 PST |   
 
+-- Should not be optimized (NULL order wrong)
+set timescaledb.debug_require_batch_sorted_merge to 'forbid';
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 ASC NULLS LAST;
              time             | x2 
 ------------------------------+----
@@ -1589,7 +1592,7 @@ SELECT * FROM test_segby ORDER BY segby, time;
          ->  Seq Scan on _timescaledb_internal.compress_hyper_8_8_chunk (actual rows=2.00 loops=1)
                Output: compress_hyper_8_8_chunk._ts_meta_count, compress_hyper_8_8_chunk.segby, compress_hyper_8_8_chunk._ts_meta_min_1, compress_hyper_8_8_chunk._ts_meta_max_1, compress_hyper_8_8_chunk._ts_meta_v2_first_time, compress_hyper_8_8_chunk._ts_meta_v2_last_time, compress_hyper_8_8_chunk."time", compress_hyper_8_8_chunk.val
 
--- Tests for #9445: forbid BatchSortedMerge on nullable orderby columns
+-- Tests for #9445: forbid BatchSortedMerge on nullable orderby columns with no firstlast index
 CREATE TABLE t(time int NOT NULL, device int, val int);
 SELECT create_hypertable('t', 'time', chunk_time_interval => 10000);
  create_hypertable 
@@ -1613,9 +1616,36 @@ SELECT compress_chunk(show_chunks('t'));
 SET timescaledb.enable_direct_compress_insert = true;
 INSERT INTO t SELECT 1, 1, g FROM generate_series(500, 800) g;
 INSERT INTO t SELECT 1, 1, g FROM generate_series(900, 1000) g;
-SET timescaledb.debug_require_batch_sorted_merge = 'forbid';
--- In DESC order NULLs come first; a NULL after a non-NULL is wrong.
+-- Batches with NULLs in orderby columns are correctly sorted with firstlast:
+-- Batch Sorted Merge is allowed
+SET timescaledb.debug_require_batch_sorted_merge = 'force';
+-- Should return 0
+SELECT count(*) AS wrong_rows FROM (
+    SELECT val, lag(val) OVER (ORDER BY val DESC) AS prev FROM t
+) t WHERE val IS NULL AND prev IS NOT NULL;
+ wrong_rows 
+------------
+          0
+
+-- Remove firstlast index from order by columns
+update _timescaledb_catalog.compression_settings
+set index = '[{"type": "minmax", "column": "val", "source": "orderby"}, {"type": "minmax", "column": "time", "source": "orderby"}]'
+where relid = 't'::regclass;
+update _timescaledb_catalog.compression_settings
+set index = '[{"type": "minmax", "column": "val", "source": "orderby"}, {"type": "minmax", "column": "time", "source": "orderby"}]'
+where compress_relid = (select format('%I.%I', schema_name, table_name)::regclass AS chunk_regclass from _timescaledb_catalog.chunk
+    where id = (select compressed_chunk_id  from _timescaledb_catalog.chunk
+        where hypertable_id = (select id from _timescaledb_catalog.hypertable
+            where table_name = 't') limit 1));
+-- Use minmax index on (val DESC, time DESC) instead
+select schema_name || '.' || table_name comp_chunk  from _timescaledb_catalog.chunk
+    where id = (select compressed_chunk_id from _timescaledb_catalog.chunk
+        where hypertable_id = (select id from _timescaledb_catalog.hypertable
+            where table_name = 't') limit 1)
+\gset
+create index compressed_index_minmax on :comp_chunk (device, _ts_meta_min_1 DESC, _ts_meta_max_1 DESC, _ts_meta_min_2 DESC, _ts_meta_max_2 DESC);
 -- Should not use BatchSortedMerge here, should return 0
+SET timescaledb.debug_require_batch_sorted_merge = 'forbid';
 SELECT count(*) AS wrong_rows FROM (
     SELECT val, lag(val) OVER (ORDER BY val DESC) AS prev FROM t
 ) t WHERE val IS NULL AND prev IS NOT NULL;
@@ -1634,6 +1664,7 @@ SELECT val, lag(val) OVER (ORDER BY val DESC NULLS FIRST) AS prev FROM t where v
   997 |  998
   996 |  997
 
+drop table t cascade;
 -- Tests for BatchSortedMerge over one-segment data
 --------------------------------------
 -- Should be optimized (all segmentby columns are pinned to a Const, orderby columns match)
@@ -1834,6 +1865,5 @@ drop table test1 cascade;
 drop table test2 cascade;
 drop table test_segby cascade;
 drop table test_nosegby cascade;
-drop table t cascade;
 RESET timescaledb.enable_direct_compress_insert;
 RESET timescaledb.debug_require_batch_sorted_merge;

tsl/test/sql/compression_sorted_merge.sql

@@ -331,11 +331,13 @@ SELECT x2, x1, c2, time FROM test1 ORDER BY time DESC;
 SELECT 1 as one, 2 as two, 3 as three, x2, x1, c2, time FROM test1 ORDER BY time DESC;
 SELECT 1 as one, 2 as two, 3 as three, x2, x1, c2, time FROM test1 ORDER BY time DESC;
 
--- Test with null values: should not optimize
-set timescaledb.debug_require_batch_sorted_merge to 'forbid';
+-- Test with null values: should optimize with firstlast index
+set timescaledb.debug_require_batch_sorted_merge to 'force';
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 ASC NULLS FIRST;
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 DESC NULLS LAST;
 
+-- should not optimize (NULL order wrong)
+set timescaledb.debug_require_batch_sorted_merge to 'forbid';
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 ASC NULLS LAST;
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 DESC NULLS FIRST;
 

tsl/test/sql/compression_sorted_merge_unordered.sql

@@ -2,6 +2,8 @@
 -- Please see the included NOTICE for copyright information and
 -- LICENSE-TIMESCALE for a copy of the license.
 
+\c :TEST_DBNAME :ROLE_SUPERUSER
+
 -- Increase the working memory limit slightly, otherwise the batch sorted merge
 -- will be penalized for segmentby cardinalities larger than 100, where it is
 -- still faster than sort.
@@ -384,12 +386,13 @@ SELECT x2, x1, c2, time FROM test1 ORDER BY time DESC;
 SELECT 1 as one, 2 as two, 3 as three, x2, x1, c2, time FROM test1 ORDER BY time DESC;
 SELECT 1 as one, 2 as two, 3 as three, x2, x1, c2, time FROM test1 ORDER BY time DESC;
 
--- Test with null values in x2
-set timescaledb.debug_require_batch_sorted_merge to 'forbid';
-
+-- Test with null values in x2: batch sorted merge supported with firstlast indexes
+set timescaledb.debug_require_batch_sorted_merge to 'force';
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 ASC NULLS FIRST;
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 DESC NULLS LAST;
 
+-- Should not be optimized (NULL order wrong)
+set timescaledb.debug_require_batch_sorted_merge to 'forbid';
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 ASC NULLS LAST;
 SELECT time, x2 FROM test_with_defined_null ORDER BY x2 DESC NULLS FIRST;
 
@@ -479,7 +482,7 @@ SELECT * FROM test_segby ORDER BY time ASC NULLS FIRST;
 :PREFIX
 SELECT * FROM test_segby ORDER BY segby, time;
 
--- Tests for #9445: forbid BatchSortedMerge on nullable orderby columns
+-- Tests for #9445: forbid BatchSortedMerge on nullable orderby columns with no firstlast index
 CREATE TABLE t(time int NOT NULL, device int, val int);
 SELECT create_hypertable('t', 'time', chunk_time_interval => 10000);
 ALTER TABLE t SET (timescaledb.compress,
@@ -498,10 +501,38 @@ SET timescaledb.enable_direct_compress_insert = true;
 INSERT INTO t SELECT 1, 1, g FROM generate_series(500, 800) g;
 INSERT INTO t SELECT 1, 1, g FROM generate_series(900, 1000) g;
 
-SET timescaledb.debug_require_batch_sorted_merge = 'forbid';
+-- Batches with NULLs in orderby columns are correctly sorted with firstlast:
+-- Batch Sorted Merge is allowed
+SET timescaledb.debug_require_batch_sorted_merge = 'force';
+
+-- Should return 0
+SELECT count(*) AS wrong_rows FROM (
+    SELECT val, lag(val) OVER (ORDER BY val DESC) AS prev FROM t
+) t WHERE val IS NULL AND prev IS NOT NULL;
+
+-- Remove firstlast index from order by columns
+update _timescaledb_catalog.compression_settings
+set index = '[{"type": "minmax", "column": "val", "source": "orderby"}, {"type": "minmax", "column": "time", "source": "orderby"}]'
+where relid = 't'::regclass;
+
+update _timescaledb_catalog.compression_settings
+set index = '[{"type": "minmax", "column": "val", "source": "orderby"}, {"type": "minmax", "column": "time", "source": "orderby"}]'
+where compress_relid = (select format('%I.%I', schema_name, table_name)::regclass AS chunk_regclass from _timescaledb_catalog.chunk
+    where id = (select compressed_chunk_id  from _timescaledb_catalog.chunk
+        where hypertable_id = (select id from _timescaledb_catalog.hypertable
+            where table_name = 't') limit 1));
+
+-- Use minmax index on (val DESC, time DESC) instead
+select schema_name || '.' || table_name comp_chunk  from _timescaledb_catalog.chunk
+    where id = (select compressed_chunk_id from _timescaledb_catalog.chunk
+        where hypertable_id = (select id from _timescaledb_catalog.hypertable
+            where table_name = 't') limit 1)
+\gset
+create index compressed_index_minmax on :comp_chunk (device, _ts_meta_min_1 DESC, _ts_meta_max_1 DESC, _ts_meta_min_2 DESC, _ts_meta_max_2 DESC);
 
--- In DESC order NULLs come first; a NULL after a non-NULL is wrong.
 -- Should not use BatchSortedMerge here, should return 0
+SET timescaledb.debug_require_batch_sorted_merge = 'forbid';
+
 SELECT count(*) AS wrong_rows FROM (
     SELECT val, lag(val) OVER (ORDER BY val DESC) AS prev FROM t
 ) t WHERE val IS NULL AND prev IS NOT NULL;
@@ -510,6 +541,8 @@ SELECT count(*) AS wrong_rows FROM (
 SET timescaledb.debug_require_batch_sorted_merge = 'force';
 SELECT val, lag(val) OVER (ORDER BY val DESC NULLS FIRST) AS prev FROM t where val > 995;
 
+drop table t cascade;
+
 -- Tests for BatchSortedMerge over one-segment data
 --------------------------------------
 
@@ -583,7 +616,6 @@ drop table test1 cascade;
 drop table test2 cascade;
 drop table test_segby cascade;
 drop table test_nosegby cascade;
-drop table t cascade;
 
 RESET timescaledb.enable_direct_compress_insert;
 RESET timescaledb.debug_require_batch_sorted_merge;