tsl/compression: fix stale sparse-index entries after rebuild_sparse_index

.unreleased/pr_10324

@@ -0,0 +1,2 @@
+Fixes: #10324 Fix stale index entries after rebuild_sparse_index() on compressed chunks
+Thanks: @tureba for reporting and fixing stale sparse-index entries after rebuild

tsl/src/compression/recompress.c

@@ -7,6 +7,7 @@
 #include <postgres.h>
 #include "debug_point.h"
 #include <access/tableam.h>
+#include <catalog/indexing.h>
 #include <miscadmin.h>
 #include <parser/parse_coerce.h>
 #include <parser/parse_relation.h>
@@ -36,6 +37,7 @@
 #include "recompress.h"
 #include "sparse_index_bloom1.h"
 #include "ts_catalog/array_utils.h"
+#include "ts_catalog/catalog.h"
 #include "ts_catalog/chunk_column_stats.h"
 #include "ts_catalog/compression_chunk_size.h"
 #include "ts_catalog/compression_settings.h"
@@ -2561,7 +2563,7 @@ populate_sparse_index_columns(Relation compressed_rel, RowDecompressor *decompre
 	TupleDesc compressed_desc = RelationGetDescr(compressed_rel);
 	TableScanDesc scan = table_beginscan_compat(compressed_rel, GetActiveSnapshot(), 0, NULL, 0);
 	TupleTableSlot *scan_slot = table_slot_create(compressed_rel, NULL);
-	TupleTableSlot *update_slot = MakeSingleTupleTableSlot(compressed_desc, &TTSOpsHeapTuple);
+	CatalogIndexState indstate = CatalogOpenIndexes(compressed_rel);
 
 	while (table_scan_getnextslot(scan, ForwardScanDirection, scan_slot))
 	{
@@ -2601,20 +2603,22 @@ populate_sparse_index_columns(Relation compressed_rel, RowDecompressor *decompre
 												decompressor->compressed_datums,
 												decompressor->compressed_is_nulls,
 												repl);
-		ExecStoreHeapTuple(new_tuple, update_slot, false);
 
 		/*
-		 * Sparse index metadata columns are not covered by any index.
-		 * If indexes on metadata columns are added in the future,
-		 * this will need to handle index updates via update_indexes.
+		 * Sparse index metadata columns are covered by a btree index
+		 * (segmentby, first/last time, and any minmax/bloom columns). If
+		 * this update isn't HOT-eligible, the old index entries are left
+		 * pointing at the superseded tuple, so we must insert new entries
+		 * ourselves -- mirroring what CatalogTupleUpdate() does for
+		 * catalog tuples. We use simple_heap_update() rather than
+		 * simple_table_tuple_update() so that heap_update() writes the new
+		 * tuple's location and HOT status directly into new_tuple, which
+		 * ts_catalog_index_insert() (a no-op when the update was HOT)
+		 * relies on below.
 		 */
 		TU_UpdateIndexes update_indexes;
-		simple_table_tuple_update(compressed_rel,
-								  &tid,
-								  update_slot,
-								  GetActiveSnapshot(),
-								  &update_indexes);
-		ExecClearTuple(update_slot);
+		simple_heap_update(compressed_rel, &tid, new_tuple, &update_indexes);
+		ts_catalog_index_insert(indstate, new_tuple);
 
 		/* Reset */
 		foreach_ptr(BatchMetadataBuilder, builder, builders)
@@ -2632,9 +2636,9 @@ populate_sparse_index_columns(Relation compressed_rel, RowDecompressor *decompre
 		}
 	}
 
-	ExecDropSingleTupleTableSlot(update_slot);
 	ExecDropSingleTupleTableSlot(scan_slot);
 	table_endscan(scan);
+	CatalogCloseIndexes(indstate);
 }
 
 void

tsl/test/expected/rebuild_sparse_index.out

@@ -518,6 +518,35 @@ FROM :ic_schema1.:ic_table1 WHERE device = 'd2' LIMIT 1;
 -------+-------+-------
  f     | f     | f
 
+-- Sparse index integrity: the (device, first_time, last_time) btree must be
+-- able to find every batch for every device after rebuild_sparse_index, not
+-- just the ones whose metadata rewrite happened to be a HOT update. Forced
+-- index/bitmap scans must return the same per-device batch counts as an
+-- unindexed scan.
+SET enable_indexscan = off;
+SET enable_bitmapscan = off;
+SELECT device, count(*) FROM :ic_schema1.:ic_table1 GROUP BY device ORDER BY device;
+ device | count 
+--------+-------
+ d1     |     2
+ d2     |     2
+ d3     |     2
+ d4     |     2
+ d5     |     2
+
+RESET enable_indexscan;
+RESET enable_bitmapscan;
+SET enable_seqscan = off;
+SELECT device, count(*) FROM :ic_schema1.:ic_table1 GROUP BY device ORDER BY device;
+ device | count 
+--------+-------
+ d1     |     2
+ d2     |     2
+ d3     |     2
+ d4     |     2
+ d5     |     2
+
+RESET enable_seqscan;
 SELECT decompress_chunk(:'ichunk1');
             decompress_chunk            
 ----------------------------------------
@@ -594,6 +623,7 @@ SELECT * FROM rsi_integrity WHERE device = 'd1' AND humidity = 42 AND label = 'l
          ->  Index Scan using _hyper_2_3_chunk_compressed_device__ts_meta_v2_first_time___idx on _hyper_2_3_chunk_compressed (actual rows=0.00 loops=1)
                Index Cond: (device = 'd1'::text)
                Filter: _timescaledb_functions.bloom1_contains_any_hashes(regress-test-bloom_577c_humidity_label, TEST-HASHES::bigint[])
+               Rows Removed by Filter: 2
    ->  Seq Scan on _hyper_2_4_chunk (actual rows=0.00 loops=1)
          Filter: ((device = 'd1'::text) AND (humidity = 42) AND (label = 'label_0'::text))
          Rows Removed by Filter: 8640

tsl/test/sql/rebuild_sparse_index.sql

@@ -355,6 +355,21 @@ SELECT _timescaledb_functions.bloom1_contains(:"bloom_label", 'nonexistent'::tex
        _timescaledb_functions.bloom1_contains(:"bloom_label", 'NOPE'::text) AS nope3
 FROM :ic_schema1.:ic_table1 WHERE device = 'd2' LIMIT 1;
 
+-- Sparse index integrity: the (device, first_time, last_time) btree must be
+-- able to find every batch for every device after rebuild_sparse_index, not
+-- just the ones whose metadata rewrite happened to be a HOT update. Forced
+-- index/bitmap scans must return the same per-device batch counts as an
+-- unindexed scan.
+SET enable_indexscan = off;
+SET enable_bitmapscan = off;
+SELECT device, count(*) FROM :ic_schema1.:ic_table1 GROUP BY device ORDER BY device;
+RESET enable_indexscan;
+RESET enable_bitmapscan;
+
+SET enable_seqscan = off;
+SELECT device, count(*) FROM :ic_schema1.:ic_table1 GROUP BY device ORDER BY device;
+RESET enable_seqscan;
+
 SELECT decompress_chunk(:'ichunk1');
 
 -- Test 11: composite bloom integrity (drop, rebuild, verify via explain)