Skip to content

Commit 88cd528

Browse files
antekresicantekresic
committed
Fix NULL deref in row_compressor_flush for segmentby cagg column
When direct compress insert is used on a hypertable with a continuous aggregate, row_compressor_flush looks up a MINMAX batch metadata builder for the cagg invalidation column to derive the invalidation range. Segmentby columns don't get a MINMAX builder (they have no _ts_meta_min/max sparse-index columns), so when the cagg time column is configured as segmentby the lookup returned NULL and the flush crashed. Fall back to reading the segmentby value directly: every row in the batch shares the same segmentby value, so it is both the min and max of the invalidation range. The remaining "no minmax and not segmentby" case shouldn't occur in a well-formed configuration, so Ensure is used to produce a clean ERROR instead of crashing if it ever does.
1 parent ac0af79 commit 88cd528

3 files changed

Lines changed: 112 additions & 28 deletions

File tree

‎tsl/src/compression/compression.c‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,7 +1567,8 @@ row_compressor_flush(RowCompressor *row_compressor, BulkWriter *writer, bool cha
15671567
if (row_compressor->invalidation)
15681568
{
15691569
InvalidationSettings *settings = row_compressor->invalidation;
1570-
AttrNumber dim_attnum = AttrOffsetGetAttrNumber(settings->invalidation_column_offset);
1570+
int16 dim_offset = settings->invalidation_column_offset;
1571+
AttrNumber dim_attnum = AttrOffsetGetAttrNumber(dim_offset);
15711572

15721573
BatchMetadataBuilderMinMax *minmax_builder = NULL;
15731574
ListCell *lc;
@@ -1585,11 +1586,32 @@ row_compressor_flush(RowCompressor *row_compressor, BulkWriter *writer, bool cha
15851586
}
15861587
}
15871588

1588-
Assert(minmax_builder != NULL);
1589-
Datum min = row_compressor->compressed_values[minmax_builder->min_metadata_attr_offset];
1590-
Datum max = row_compressor->compressed_values[minmax_builder->max_metadata_attr_offset];
1591-
int64 start = ts_time_value_to_internal(min, minmax_builder->type_oid);
1592-
int64 end = ts_time_value_to_internal(max, minmax_builder->type_oid);
1589+
Datum min;
1590+
Datum max;
1591+
Oid type_oid;
1592+
if (minmax_builder != NULL)
1593+
{
1594+
min = row_compressor->compressed_values[minmax_builder->min_metadata_attr_offset];
1595+
max = row_compressor->compressed_values[minmax_builder->max_metadata_attr_offset];
1596+
type_oid = minmax_builder->type_oid;
1597+
}
1598+
else
1599+
{
1600+
/*
1601+
* No minmax sparse index for the cagg invalidation column means it
1602+
* is configured as segmentby. Every row in the batch shares the
1603+
* same segmentby value, so use it as both bounds.
1604+
*/
1605+
PerColumn *dim_column = &row_compressor->per_column[dim_offset];
1606+
Ensure(dim_column->compressor == NULL && dim_column->segment_info != NULL,
1607+
"cagg invalidation column has no minmax builder and is not segmentby");
1608+
Ensure(!dim_column->segment_info->is_null, "cagg invalidation column is NULL");
1609+
int16 compressed_col = row_compressor->uncompressed_col_to_compressed_col[dim_offset];
1610+
min = max = row_compressor->compressed_values[compressed_col];
1611+
type_oid = TupleDescAttr(row_compressor->in_desc, dim_offset)->atttypid;
1612+
}
1613+
int64 start = ts_time_value_to_internal(min, type_oid);
1614+
int64 end = ts_time_value_to_internal(max, type_oid);
15931615
continuous_agg_invalidate_range(settings->hypertable_id, settings->chunk_relid, start, end);
15941616
}
15951617

‎tsl/test/expected/direct_compress_insert.out‎

Lines changed: 56 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,40 @@ SELECT DISTINCT status FROM _timescaledb_catalog.chunk WHERE compressed_chunk_id
435435
--------
436436
3
437437

438+
ROLLBACK;
439+
-- cagg + direct compress where the cagg time column is segmentby.
440+
-- segmentby columns don't get _ts_meta_min/max columns, so no MINMAX builder
441+
-- is created for them; the flush path must still derive an invalidation range
442+
-- from the segmentby value rather than crashing.
443+
BEGIN;
444+
CREATE TABLE metrics_segmentby_time (time TIMESTAMPTZ NOT NULL, device TEXT, value float)
445+
WITH (tsdb.hypertable);
446+
NOTICE: using column "time" as partitioning column
447+
CREATE MATERIALIZED VIEW metrics_segmentby_time_cagg WITH (tsdb.continuous) AS
448+
SELECT time_bucket('1 hour', time) AS bucket, device, avg(value) AS avg_value
449+
FROM metrics_segmentby_time GROUP BY bucket, device WITH NO DATA;
450+
ALTER TABLE metrics_segmentby_time SET (
451+
timescaledb.compress,
452+
timescaledb.compress_segmentby = 'time'
453+
);
454+
NOTICE: updated compression settings will only apply to future compressions
455+
INSERT INTO metrics_segmentby_time
456+
SELECT '2025-01-01'::timestamptz + (i * interval '5 minutes'), 'd1', random()
457+
FROM generate_series(1, 50) i;
458+
SET timescaledb.enable_direct_compress_insert = true;
459+
INSERT INTO metrics_segmentby_time
460+
SELECT '2025-02-01'::timestamptz + (i * interval '1 second'), 'd1', random()
461+
FROM generate_series(1, 5000) i;
462+
-- The new chunk should be in COMPRESSED state, confirming the flush path ran.
463+
SELECT _timescaledb_functions.chunk_status_text(chunk)
464+
FROM show_chunks('metrics_segmentby_time') chunk
465+
WHERE chunk::text LIKE '%hyper_%'
466+
ORDER BY chunk::text;
467+
chunk_status_text
468+
------------------------
469+
{}
470+
{COMPRESSED,UNORDERED}
471+
438472
ROLLBACK;
439473
-- test chunk status handling
440474
CREATE TABLE metrics_status(time timestamptz) WITH (tsdb.hypertable,tsdb.partition_column='time');
@@ -514,7 +548,7 @@ SELECT _timescaledb_functions.chunk_status_text(chunk) FROM show_chunks('metrics
514548
SELECT compress_chunk(show_chunks('metrics_status'));
515549
compress_chunk
516550
-----------------------------------------
517-
_timescaledb_internal._hyper_4_54_chunk
551+
_timescaledb_internal._hyper_7_57_chunk
518552

519553
-- status should be COMPRESSED
520554
SELECT _timescaledb_functions.chunk_status_text(chunk) FROM show_chunks('metrics_status') chunk;
@@ -563,7 +597,7 @@ EXPLAIN (costs off,summary off,timing off) INSERT INTO :CHUNK SELECT '2025-01-01
563597
--- QUERY PLAN ---
564598
Custom Scan (ModifyHypertable)
565599
Direct Compress: true
566-
-> Insert on _hyper_6_56_chunk
600+
-> Insert on _hyper_9_59_chunk
567601
-> Function Scan on generate_series i
568602

569603
BEGIN;
@@ -586,8 +620,8 @@ EXPLAIN (analyze,buffers off,costs off,summary off,timing off) DELETE FROM :CHUN
586620
Batches scanned: 1
587621
Batches decompressed: 1
588622
Tuples decompressed: 101
589-
-> Delete on _hyper_6_56_chunk (actual rows=0.00 loops=1)
590-
-> Index Scan using _hyper_6_56_chunk_metrics_chunk_time_idx on _hyper_6_56_chunk (actual rows=100.00 loops=1)
623+
-> Delete on _hyper_9_59_chunk (actual rows=0.00 loops=1)
624+
-> Index Scan using _hyper_9_59_chunk_metrics_chunk_time_idx on _hyper_9_59_chunk (actual rows=100.00 loops=1)
591625
Index Cond: ("time" > 'Wed Jan 01 00:00:00 2025 PST'::timestamp with time zone)
592626

593627
SELECT count(*) FROM :CHUNK;
@@ -615,8 +649,8 @@ SET timescaledb.direct_compress_insert_tuple_sort_limit = 1000;
615649
INSERT INTO metrics SELECT '2025-01-02'::timestamptz + (i || ' minute')::interval, 'd1', i::float FROM generate_series(1,3000) i;
616650
EXPLAIN (ANALYZE, BUFFERS OFF, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT * FROM metrics;
617651
--- QUERY PLAN ---
618-
Custom Scan (ColumnarScan) on _hyper_1_59_chunk (actual rows=3000.00 loops=1)
619-
-> Seq Scan on compress_hyper_2_60_chunk (actual rows=3.00 loops=1)
652+
Custom Scan (ColumnarScan) on _hyper_1_62_chunk (actual rows=3000.00 loops=1)
653+
-> Seq Scan on compress_hyper_2_63_chunk (actual rows=3.00 loops=1)
620654

621655
SELECT first(time,rn), last(time,rn) FROM (SELECT ROW_NUMBER() OVER () as rn, time FROM metrics) sub;
622656
first | last
@@ -694,18 +728,18 @@ ANALYZE test_segmentby_stats;
694728
SELECT compress_chunk(c) FROM show_chunks('test_segmentby_stats') c;
695729
compress_chunk
696730
------------------------------------------
697-
_timescaledb_internal._hyper_12_89_chunk
731+
_timescaledb_internal._hyper_15_92_chunk
698732

699733
-- will have devide_id as default segmentby for compressed chunk;
700734
SELECT * FROM _timescaledb_catalog.compression_settings;
701735
relid | compress_relid | segmentby | orderby | orderby_desc | orderby_nullsfirst | index
702736
------------------------------------------+--------------------------------------------------+-------------+---------+--------------+--------------------+-------------------------------------------------------------
703737
metrics | | | {time} | {f} | {f} | [{"type": "minmax", "column": "time", "source": "orderby"}]
704738
metrics_status | | | | | |
705-
_timescaledb_internal._hyper_4_54_chunk | _timescaledb_internal.compress_hyper_5_55_chunk | | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
739+
_timescaledb_internal._hyper_7_57_chunk | _timescaledb_internal.compress_hyper_8_58_chunk | | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
706740
metrics_chunk | | | | | |
707741
test_segmentby_stats | | | | | |
708-
_timescaledb_internal._hyper_12_89_chunk | _timescaledb_internal.compress_hyper_13_90_chunk | {device_id} | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
742+
_timescaledb_internal._hyper_15_92_chunk | _timescaledb_internal.compress_hyper_16_93_chunk | {device_id} | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
709743

710744
SET timescaledb.enable_direct_compress_insert = true;
711745
INSERT INTO test_segmentby_stats
@@ -719,11 +753,11 @@ SELECT * FROM _timescaledb_catalog.compression_settings;
719753
------------------------------------------+--------------------------------------------------+-------------+---------+--------------+--------------------+-------------------------------------------------------------
720754
metrics | | | {time} | {f} | {f} | [{"type": "minmax", "column": "time", "source": "orderby"}]
721755
metrics_status | | | | | |
722-
_timescaledb_internal._hyper_4_54_chunk | _timescaledb_internal.compress_hyper_5_55_chunk | | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
756+
_timescaledb_internal._hyper_7_57_chunk | _timescaledb_internal.compress_hyper_8_58_chunk | | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
723757
metrics_chunk | | | | | |
724758
test_segmentby_stats | | | | | |
725-
_timescaledb_internal._hyper_12_89_chunk | _timescaledb_internal.compress_hyper_13_90_chunk | {device_id} | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
726-
_timescaledb_internal._hyper_12_91_chunk | _timescaledb_internal.compress_hyper_13_93_chunk | {device_id} | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
759+
_timescaledb_internal._hyper_15_92_chunk | _timescaledb_internal.compress_hyper_16_93_chunk | {device_id} | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
760+
_timescaledb_internal._hyper_15_94_chunk | _timescaledb_internal.compress_hyper_16_96_chunk | {device_id} | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
727761

728762
ROLLBACK;
729763
BEGIN;
@@ -746,10 +780,10 @@ SELECT * FROM _timescaledb_catalog.compression_settings;
746780
------------------------------------------+--------------------------------------------------+-------------+---------+--------------+--------------------+-------------------------------------------------------------
747781
metrics | | | {time} | {f} | {f} | [{"type": "minmax", "column": "time", "source": "orderby"}]
748782
metrics_status | | | | | |
749-
_timescaledb_internal._hyper_4_54_chunk | _timescaledb_internal.compress_hyper_5_55_chunk | | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
783+
_timescaledb_internal._hyper_7_57_chunk | _timescaledb_internal.compress_hyper_8_58_chunk | | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
750784
metrics_chunk | | | | | |
751785
test_segmentby_stats | | {device_id} | | | |
752-
_timescaledb_internal._hyper_14_94_chunk | _timescaledb_internal.compress_hyper_15_95_chunk | {device_id} | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
786+
_timescaledb_internal._hyper_17_97_chunk | _timescaledb_internal.compress_hyper_18_98_chunk | {device_id} | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
753787

754788
ROLLBACK;
755789
-- Test orderby columns are not selected by DC segmentby default
@@ -767,13 +801,13 @@ SET timescaledb.enable_direct_compress_insert = on;
767801
INSERT INTO test_orderby_not_segmentby SELECT '2024-06-01'::timestamptz + (i||' min')::interval,
768802
(i%5)+1, random() FROM generate_series(1,2000) i;
769803
SELECT * FROM _timescaledb_catalog.compression_settings;
770-
relid | compress_relid | segmentby | orderby | orderby_desc | orderby_nullsfirst | index
771-
------------------------------------------+--------------------------------------------------+-----------+------------------+--------------+--------------------+-----------------------------------------------------------------------------------------------------------------------------
772-
metrics | | | {time} | {f} | {f} | [{"type": "minmax", "column": "time", "source": "orderby"}]
773-
metrics_status | | | | | |
774-
_timescaledb_internal._hyper_4_54_chunk | _timescaledb_internal.compress_hyper_5_55_chunk | | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
775-
metrics_chunk | | | | | |
776-
test_orderby_not_segmentby | | | | | |
777-
_timescaledb_internal._hyper_16_97_chunk | _timescaledb_internal.compress_hyper_17_98_chunk | | {time,device_id} | {t,f} | {t,f} | [{"type": "minmax", "column": "time", "source": "orderby"}, {"type": "minmax", "column": "device_id", "source": "orderby"}]
804+
relid | compress_relid | segmentby | orderby | orderby_desc | orderby_nullsfirst | index
805+
-------------------------------------------+---------------------------------------------------+-----------+------------------+--------------+--------------------+-----------------------------------------------------------------------------------------------------------------------------
806+
metrics | | | {time} | {f} | {f} | [{"type": "minmax", "column": "time", "source": "orderby"}]
807+
metrics_status | | | | | |
808+
_timescaledb_internal._hyper_7_57_chunk | _timescaledb_internal.compress_hyper_8_58_chunk | | {time} | {t} | {t} | [{"type": "minmax", "column": "time", "source": "orderby"}]
809+
metrics_chunk | | | | | |
810+
test_orderby_not_segmentby | | | | | |
811+
_timescaledb_internal._hyper_19_100_chunk | _timescaledb_internal.compress_hyper_20_101_chunk | | {time,device_id} | {t,f} | {t,f} | [{"type": "minmax", "column": "time", "source": "orderby"}, {"type": "minmax", "column": "device_id", "source": "orderby"}]
778812

779813
ROLLBACK;

‎tsl/test/sql/direct_compress_insert.sql‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,34 @@ EXPLAIN (ANALYZE, BUFFERS OFF, COSTS OFF, SUMMARY OFF, TIMING OFF) SELECT * FROM
208208
SELECT DISTINCT status FROM _timescaledb_catalog.chunk WHERE compressed_chunk_id IS NOT NULL;
209209
ROLLBACK;
210210

211+
-- cagg + direct compress where the cagg time column is segmentby.
212+
-- segmentby columns don't get _ts_meta_min/max columns, so no MINMAX builder
213+
-- is created for them; the flush path must still derive an invalidation range
214+
-- from the segmentby value rather than crashing.
215+
BEGIN;
216+
CREATE TABLE metrics_segmentby_time (time TIMESTAMPTZ NOT NULL, device TEXT, value float)
217+
WITH (tsdb.hypertable);
218+
CREATE MATERIALIZED VIEW metrics_segmentby_time_cagg WITH (tsdb.continuous) AS
219+
SELECT time_bucket('1 hour', time) AS bucket, device, avg(value) AS avg_value
220+
FROM metrics_segmentby_time GROUP BY bucket, device WITH NO DATA;
221+
ALTER TABLE metrics_segmentby_time SET (
222+
timescaledb.compress,
223+
timescaledb.compress_segmentby = 'time'
224+
);
225+
INSERT INTO metrics_segmentby_time
226+
SELECT '2025-01-01'::timestamptz + (i * interval '5 minutes'), 'd1', random()
227+
FROM generate_series(1, 50) i;
228+
SET timescaledb.enable_direct_compress_insert = true;
229+
INSERT INTO metrics_segmentby_time
230+
SELECT '2025-02-01'::timestamptz + (i * interval '1 second'), 'd1', random()
231+
FROM generate_series(1, 5000) i;
232+
-- The new chunk should be in COMPRESSED state, confirming the flush path ran.
233+
SELECT _timescaledb_functions.chunk_status_text(chunk)
234+
FROM show_chunks('metrics_segmentby_time') chunk
235+
WHERE chunk::text LIKE '%hyper_%'
236+
ORDER BY chunk::text;
237+
ROLLBACK;
238+
211239
-- test chunk status handling
212240
CREATE TABLE metrics_status(time timestamptz) WITH (tsdb.hypertable,tsdb.partition_column='time');
213241

0 commit comments

Comments
 (0)