Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 20 additions & 19 deletions tsl/src/nodes/columnar_scan/columnar_scan.c
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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++)
Expand Down Expand Up @@ -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))
{
Expand All @@ -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;
}
Expand Down Expand Up @@ -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,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a note for the future, I hope we can make the meaning of this more explicit. As far as I can understand, for full compressed sort pushdown the requirement is: "compressed sort leads to all batch rows beign fully sorted in required order", and for batch sorted merge it is: "compressed sort leads to first batch rows being sorted in required order"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the "all rows sorted" condition is basically the "first row sorted" condition + "batches are not overlapping".

&sort_info.reverse);
}
return sort_info;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
6 changes: 4 additions & 2 deletions tsl/test/expected/compression_sorted_merge.out
Original file line number Diff line number Diff line change
Expand Up @@ -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
------------------------------+----
Expand All @@ -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
------------------------------+----
Expand Down
42 changes: 36 additions & 6 deletions tsl/test/expected/compression_sorted_merge_unordered.out
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
------------------------------+----
Expand Down Expand Up @@ -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
------------------------------+----
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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;
6 changes: 4 additions & 2 deletions tsl/test/sql/compression_sorted_merge.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
46 changes: 39 additions & 7 deletions tsl/test/sql/compression_sorted_merge_unordered.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand All @@ -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
--------------------------------------

Expand Down Expand Up @@ -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;
Loading