From 7c184e4d681c6c4b91e589a842fc3144cc8b636e Mon Sep 17 00:00:00 2001 From: Natalya Aksman Date: Wed, 3 Jun 2026 15:23:58 -0400 Subject: [PATCH] Enable Batch Sorted Merge for nullable order by with firstlast index --- tsl/src/nodes/columnar_scan/columnar_scan.c | 39 ++++++++-------- .../expected/compression_sorted_merge.out | 6 ++- .../compression_sorted_merge_unordered.out | 42 ++++++++++++++--- tsl/test/sql/compression_sorted_merge.sql | 6 ++- .../compression_sorted_merge_unordered.sql | 46 ++++++++++++++++--- 5 files changed, 103 insertions(+), 36 deletions(-) diff --git a/tsl/src/nodes/columnar_scan/columnar_scan.c b/tsl/src/nodes/columnar_scan/columnar_scan.c index 0faa4c3f5f0..3a09f51c3c1 100644 --- a/tsl/src/nodes/columnar_scan/columnar_scan.c +++ b/tsl/src/nodes/columnar_scan/columnar_scan.c @@ -2865,6 +2865,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 @@ -2916,8 +2918,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++) @@ -2946,13 +2948,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)) { @@ -2967,9 +2969,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; } @@ -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; @@ -3231,7 +3231,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; @@ -3247,12 +3247,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; } diff --git a/tsl/test/expected/compression_sorted_merge.out b/tsl/test/expected/compression_sorted_merge.out index 2849ff9a381..ca211d10eb8 100644 --- a/tsl/test/expected/compression_sorted_merge.out +++ b/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 ------------------------------+---- diff --git a/tsl/test/expected/compression_sorted_merge_unordered.out b/tsl/test/expected/compression_sorted_merge_unordered.out index bb35c9aeef2..74116f6f84d 100644 --- a/tsl/test/expected/compression_sorted_merge_unordered.out +++ b/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; diff --git a/tsl/test/sql/compression_sorted_merge.sql b/tsl/test/sql/compression_sorted_merge.sql index 9047c0d8c4c..0683684767b 100644 --- a/tsl/test/sql/compression_sorted_merge.sql +++ b/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; diff --git a/tsl/test/sql/compression_sorted_merge_unordered.sql b/tsl/test/sql/compression_sorted_merge_unordered.sql index 3900c3434b6..1c58f9dab26 100644 --- a/tsl/test/sql/compression_sorted_merge_unordered.sql +++ b/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;