Fix chunk skipping min/max calculation

When a partially compressed chunk with segmentby column was
recompressed the new min/max range would not be calculated correctly
and only take the new values into account but not already compressed
values.

Author: svenklemm

.unreleased/pr_8426

@@ -0,0 +1 @@
+Fixes: #8426 Fix chunk skipping min/max calculation

src/chunk_adaptive.c

@@ -276,9 +276,9 @@ table_has_minmax_index(Oid relid, Oid atttype, Name attname, AttrNumber attnum)
  *
  * Returns true iff min and max is found, otherwise false.
  */
-bool
-ts_chunk_get_minmax(Oid relid, Oid atttype, AttrNumber attnum, const char *call_context,
-					Datum minmax[2])
+static bool
+chunk_get_minmax(Oid relid, Oid atttype, AttrNumber attnum, const char *call_context,
+				 Datum minmax[2])
 {
 	Relation rel = table_open(relid, AccessShareLock);
 	NameData attname;
@@ -484,11 +484,11 @@ ts_calculate_chunk_interval(PG_FUNCTION_ARGS)
 
 		slice_interval = slice->fd.range_end - slice->fd.range_start;
 
-		if (ts_chunk_get_minmax(chunk->table_id,
-								dim->fd.column_type,
-								attno,
-								"adaptive chunking",
-								minmax))
+		if (chunk_get_minmax(chunk->table_id,
+							 dim->fd.column_type,
+							 attno,
+							 "adaptive chunking",
+							 minmax))
 		{
 			int64 min = ts_time_value_to_internal(minmax[0], dim->fd.column_type);
 			int64 max = ts_time_value_to_internal(minmax[1], dim->fd.column_type);

src/chunk_adaptive.h

@@ -26,8 +26,6 @@ typedef struct ChunkSizingInfo
 
 extern void ts_chunk_adaptive_sizing_info_validate(ChunkSizingInfo *info);
 extern void ts_chunk_sizing_func_validate(regproc func, ChunkSizingInfo *info);
-extern bool ts_chunk_get_minmax(Oid relid, Oid atttype, AttrNumber attnum, const char *call_context,
-								Datum minmax[2]);
 extern TSDLLEXPORT ChunkSizingInfo *ts_chunk_sizing_info_get_default_disabled(Oid table_relid);
 
 extern TSDLLEXPORT int64 ts_chunk_calculate_initial_chunk_target_size(void);

src/ts_catalog/chunk_column_stats.c

@@ -11,6 +11,7 @@
 #include <access/stratnum.h>
 #include <access/tupdesc.h>
 #include <catalog/pg_collation.h>
+#include <executor/spi.h>
 #include <executor/tuptable.h>
 #include <funcapi.h>
 #include <nodes/makefuncs.h>
@@ -22,6 +23,7 @@
 #include <rewrite/rewriteManip.h>
 #include <storage/lmgr.h>
 #include <storage/lockdefs.h>
+#include <utils/datum.h>
 #include <utils/syscache.h>
 
 #include "chunk.h"
@@ -781,6 +783,76 @@ ts_chunk_column_stats_lookup(int32 hypertable_id, int32 chunk_id, const char *co
 	return form_range;
 }
 
+static bool
+chunk_get_minmax(const Chunk *chunk, Oid col_type, const char *col_name, Datum *minmax)
+{
+	StringInfoData command;
+	int res;
+
+	/* Lock down search_path */
+	int save_nestlevel = NewGUCNestLevel();
+	RestrictSearchPath();
+
+	initStringInfo(&command);
+	appendStringInfo(&command,
+					 "SELECT pg_catalog.min(%s), pg_catalog.max(%s) FROM %s.%s",
+					 quote_identifier(col_name),
+					 quote_identifier(col_name),
+					 quote_identifier(NameStr(chunk->fd.schema_name)),
+					 quote_identifier(NameStr(chunk->fd.table_name)));
+
+	/*
+	 * SPI_connect will switch MemoryContext so we need to keep track
+	 * of caller context as we need to copy the values into caller
+	 * context.
+	 */
+	MemoryContext caller = CurrentMemoryContext;
+
+	if (SPI_connect() != SPI_OK_CONNECT)
+		elog(ERROR, "could not connect to SPI");
+
+	res = SPI_execute(command.data, true /* read_only */, 0 /*count*/);
+	if (res < 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_INTERNAL_ERROR),
+				 (errmsg("could not get the min/max values for column \"%s\" of chunk \"%s.%s\"",
+						 col_name,
+						 chunk->fd.schema_name.data,
+						 chunk->fd.table_name.data))));
+
+	pfree(command.data);
+
+	Datum min, max;
+	bool isnull_min = false, isnull_max = false;
+	min = SPI_getbinval(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 1, &isnull_min);
+	max = SPI_getbinval(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 2, &isnull_max);
+	Assert(SPI_gettypeid(SPI_tuptable->tupdesc, 1) == col_type);
+	Assert(SPI_gettypeid(SPI_tuptable->tupdesc, 2) == col_type);
+
+	bool found = !isnull_min && !isnull_max;
+	if (found)
+	{
+		bool typbyval;
+		int16 typlen;
+		get_typlenbyval(col_type, &typlen, &typbyval);
+
+		/* Copy the values into caller context */
+		MemoryContext spi = MemoryContextSwitchTo(caller);
+		minmax[0] = datumCopy(min, typbyval, typlen);
+		minmax[1] = datumCopy(max, typbyval, typlen);
+		MemoryContextSwitchTo(spi);
+	}
+
+	/* Restore search_path */
+	AtEOXact_GUC(false, save_nestlevel);
+
+	res = SPI_finish();
+	if (res != SPI_OK_FINISH)
+		elog(ERROR, "SPI_finish failed: %s", SPI_result_code_string(res));
+
+	return found;
+}
+
 /*
  * Update column dimension ranges in the catalog for the
  * provided chunk (it's assumed that the chunk is locked
@@ -821,7 +893,7 @@ ts_chunk_column_stats_calculate(const Hypertable *ht, const Chunk *chunk)
 		col_type = get_atttype(chunk->table_id, attno);
 
 		/* calculate the min/max range for this column on this chunk */
-		if (ts_chunk_get_minmax(chunk->table_id, col_type, attno, "column range", minmax))
+		if (chunk_get_minmax(chunk, col_type, col_name, minmax))
 		{
 			Form_chunk_column_stats range;
 			int64 min = ts_time_value_to_internal(minmax[0], col_type);

tsl/src/compression/api.c

@@ -486,28 +486,12 @@ compress_chunk_impl(Oid hypertable_relid, Oid chunk_relid)
 
 	before_size = ts_relation_size_impl(cxt.srcht_chunk->table_id);
 
-	/*
-	 * Calculate and add the column dimension ranges for the src chunk. This has to
-	 * be done before the compression. In case of recompression, the logic will get the
-	 * min/max entries for the uncompressed portion and reconcile and update the existing
-	 * entry for ht/chunk/column combination. This case handles:
-	 *
-	 * * INSERTs into uncompressed chunk
-	 * * UPDATEs into uncompressed chunk
-	 *
-	 * In case of DELETEs, the entries won't exist in the uncompressed chunk, but since
-	 * we are deleting, we will stay within the earlier computed max/min range. This
-	 * means that the chunk will not get pruned for a larger range of values. This will
-	 * work ok enough if only a few of the compressed chunks get DELETEs down the line.
-	 * In the future, we can look at computing min/max entries in the compressed chunk
-	 * using the batch metadata and then recompute the range to handle DELETE cases.
-	 */
-	if (cxt.srcht->range_space)
-		ts_chunk_column_stats_calculate(cxt.srcht, cxt.srcht_chunk);
-
 	cstat = compress_chunk(cxt.srcht_chunk->table_id, compress_ht_chunk->table_id, insert_options);
 	after_size = ts_relation_size_impl(compress_ht_chunk->table_id);
 
+	if (cxt.srcht->range_space)
+		ts_chunk_column_stats_calculate(cxt.srcht, cxt.srcht_chunk);
+
 	if (new_compressed_chunk)
 	{
 		compression_chunk_size_catalog_insert(cxt.srcht_chunk->fd.id,

tsl/src/compression/recompress.c

@@ -184,22 +184,6 @@ recompress_chunk_segmentwise_impl(Chunk *uncompressed_chunk)
 		}
 	}
 
-	/*
-	 * Calculate and add the column dimension ranges for the src chunk used by chunk skipping
-	 * feature. This has to be done before the compression. In case of recompression, the logic will
-	 * get the min/max entries for the uncompressed portion and reconcile and update the existing
-	 * entry for ht/chunk/column combination. This case handles:
-	 *
-	 * * INSERTs into uncompressed chunk
-	 * * UPDATEs into uncompressed chunk
-	 *
-	 * In case of DELETEs, the entries won't exist in the uncompressed chunk, but since
-	 * we are deleting, we will stay within the earlier computed max/min range. This
-	 * means that the chunk will not get pruned for a larger range of values. This will
-	 * work ok enough if only a few of the compressed chunks get DELETEs down the line.
-	 * In the future, we can look at computing min/max entries in the compressed chunk
-	 * using the batch metadata and then recompute the range to handle DELETE cases.
-	 */
 	Hypertable *ht = ts_hypertable_get_by_id(uncompressed_chunk->fd.hypertable_id);
 	if (ht->range_space)
 		ts_chunk_column_stats_calculate(ht, uncompressed_chunk);

tsl/test/expected/chunk_column_stats.out

@@ -185,7 +185,6 @@ SELECT * from _timescaledb_catalog.chunk_column_stats WHERE chunk_id = :'CHUNK_I
 DROP INDEX sense_idx;
 -- recompress the partial chunk
 SELECT compress_chunk(:'CH_NAME');
-WARNING:  no index on "sensor_id" found for column range on chunk "_hyper_1_1_chunk"
              compress_chunk             
 ----------------------------------------
  _timescaledb_internal._hyper_1_1_chunk
@@ -201,6 +200,12 @@ WHERE hypertable_name = 'sample_table' AND chunk_name = :'CH_NAME';
 (1 row)
 
 -- The chunk entry should become "valid" again
+SELECT min(sensor_id), max(sensor_id) FROM :CH_NAME;
+ min | max 
+-----+-----
+   1 |   8
+(1 row)
+
 SELECT * from _timescaledb_catalog.chunk_column_stats WHERE chunk_id = :'CHUNK_ID';
  id | hypertable_id | chunk_id | column_name | range_start | range_end | valid 
 ----+---------------+----------+-------------+-------------+-----------+-------
@@ -632,7 +637,6 @@ SELECT * from _timescaledb_catalog.chunk_column_stats;
 
 -- Compressing a chunk again should calculate proper ranges
 SELECT compress_chunk(:'CH_NAME');
-WARNING:  no index on "sensor_id" found for column range on chunk "_hyper_1_1_chunk"
              compress_chunk             
 ----------------------------------------
  _timescaledb_internal._hyper_1_1_chunk
@@ -665,7 +669,6 @@ SELECT * from _timescaledb_catalog.chunk_column_stats;
 
 -- Check that truncate resets the entry in the catalog
 SELECT compress_chunk(:'CH_NAME');
-WARNING:  no index on "sensor_id" found for column range on chunk "_hyper_1_1_chunk"
              compress_chunk             
 ----------------------------------------
  _timescaledb_internal._hyper_1_1_chunk
@@ -820,3 +823,39 @@ SELECT * FROM _timescaledb_catalog.chunk_column_stats;
  12 |             4 |        8 | temperature |                  366 |                 502 | t
 (2 rows)
 
+-- Check min/max ranges for partial chunks with segmentby columns get recalculated correctly by seementwise recompression
+CREATE TABLE chunk_skipping(time timestamptz,device text, updated_at timestamptz)
+WITH (tsdb.hypertable, tsdb.partition_column='time',tsdb.segmentby='device');
+NOTICE:  adding not-null constraint to column "time"
+SELECT enable_chunk_skipping('chunk_skipping', 'updated_at');
+ enable_chunk_skipping 
+-----------------------
+ (13,t)
+(1 row)
+
+INSERT INTO chunk_skipping SELECT '2025-01-01', 'd1', '2025-01-01';
+SELECT compress_chunk(show_chunks('chunk_skipping'));
+             compress_chunk              
+-----------------------------------------
+ _timescaledb_internal._hyper_6_10_chunk
+(1 row)
+
+SELECT * from chunk_skipping where updated_at < '2026-01-01';
+             time             | device |          updated_at          
+------------------------------+--------+------------------------------
+ Wed Jan 01 00:00:00 2025 PST | d1     | Wed Jan 01 00:00:00 2025 PST
+(1 row)
+
+INSERT INTO chunk_skipping SELECT '2025-01-01', 'd2', '2026-01-01';
+SELECT compress_chunk(show_chunks('chunk_skipping'));
+             compress_chunk              
+-----------------------------------------
+ _timescaledb_internal._hyper_6_10_chunk
+(1 row)
+
+SELECT * from chunk_skipping where updated_at < '2026-01-01';
+             time             | device |          updated_at          
+------------------------------+--------+------------------------------
+ Wed Jan 01 00:00:00 2025 PST | d1     | Wed Jan 01 00:00:00 2025 PST
+(1 row)
+

tsl/test/sql/chunk_column_stats.sql

@@ -135,6 +135,7 @@ FROM compressed_chunk_info_view
 WHERE hypertable_name = 'sample_table' AND chunk_name = :'CH_NAME';
 
 -- The chunk entry should become "valid" again
+SELECT min(sensor_id), max(sensor_id) FROM :CH_NAME;
 SELECT * from _timescaledb_catalog.chunk_column_stats WHERE chunk_id = :'CHUNK_ID';
 
 -- A query using a WHERE clause on "sensor_id" column will scan the proper chunk
@@ -310,3 +311,19 @@ SELECT enable_chunk_skipping('sample_table', 'temperature');
 SELECT show_chunks('sample_table') AS "CH_NAME" order by 1 limit 1 \gset
 SELECT compress_chunk(:'CH_NAME');
 SELECT * FROM _timescaledb_catalog.chunk_column_stats;
+
+-- Check min/max ranges for partial chunks with segmentby columns get recalculated correctly by seementwise recompression
+CREATE TABLE chunk_skipping(time timestamptz,device text, updated_at timestamptz)
+WITH (tsdb.hypertable, tsdb.partition_column='time',tsdb.segmentby='device');
+
+SELECT enable_chunk_skipping('chunk_skipping', 'updated_at');
+
+INSERT INTO chunk_skipping SELECT '2025-01-01', 'd1', '2025-01-01';
+SELECT compress_chunk(show_chunks('chunk_skipping'));
+
+SELECT * from chunk_skipping where updated_at < '2026-01-01';
+
+INSERT INTO chunk_skipping SELECT '2025-01-01', 'd2', '2026-01-01';
+SELECT compress_chunk(show_chunks('chunk_skipping'));
+
+SELECT * from chunk_skipping where updated_at < '2026-01-01';