Fix bitmap table scan crash on iterator release

melanieplageman · melanieplageman · commit 754c610e13b8 · 2024-12-19T11:55:03.000-05:00
1a0da34 replaced Bitmap Table Scan's individual private and shared iterators with a unified iterator. It neglected, however, to check if the iterator had already been cleaned up before doing so on rescan. Add this check both on rescan and end scan to be safe. Reported-by: Richard Guo Author: Richard Guo Discussion: https://postgr.es/m/CAMbWs48nrhcLY1kcd-u9oD%2B6yiS631F_8Fx8ZGsO-BYDwH%2Bbyw%40mail.gmail.com
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
@@ -572,9 +572,11 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	if (scan)
 	{
 		/*
-		 * End iteration on iterators saved in scan descriptor.
+		 * End iteration on iterators saved in scan descriptor if they have
+		 * not already been cleaned up.
 		 */
-		tbm_end_iterate(&scan->st.rs_tbmiterator);
+		if (!tbm_exhausted(&scan->st.rs_tbmiterator))
+			tbm_end_iterate(&scan->st.rs_tbmiterator);
 
 		/* rescan to release any page pin */
 		table_rescan(node->ss.ss_currentScanDesc, NULL);
@@ -654,9 +656,11 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	if (scanDesc)
 	{
 		/*
-		 * End iteration on iterators saved in scan descriptor.
+		 * End iteration on iterators saved in scan descriptor if they have
+		 * not already been cleaned up.
 		 */
-		tbm_end_iterate(&scanDesc->st.rs_tbmiterator);
+		if (!tbm_exhausted(&scanDesc->st.rs_tbmiterator))
+			tbm_end_iterate(&scanDesc->st.rs_tbmiterator);
 
 		/*
 		 * close table scan
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
@@ -1594,7 +1594,7 @@ tbm_begin_iterate(TIDBitmap *tbm, dsa_area *dsa, dsa_pointer dsp)
 void
 tbm_end_iterate(TBMIterator *iterator)
 {
-	Assert(iterator);
+	Assert(iterator && !tbm_exhausted(iterator));
 
 	if (iterator->shared)
 		tbm_end_shared_iterate(iterator->i.shared_iterator);
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
@@ -8182,3 +8182,38 @@ SELECT t1.a FROM skip_fetch t1 LEFT JOIN skip_fetch t2 ON t2.a = 1 WHERE t2.a IS
 
 RESET enable_indexonlyscan;
 RESET enable_seqscan;
+-- Test BitmapHeapScan with a rescan releases resources correctly
+SET enable_seqscan = off;
+SET enable_indexscan = off;
+CREATE TEMP TABLE rescan_bhs (a INT);
+INSERT INTO rescan_bhs VALUES (1), (2);
+CREATE INDEX ON rescan_bhs (a);
+EXPLAIN (COSTS OFF)
+SELECT * FROM rescan_bhs t1 LEFT JOIN rescan_bhs t2 ON t1.a IN
+  (SELECT a FROM rescan_bhs t3 WHERE t2.a > 1);
+                        QUERY PLAN                         
+-----------------------------------------------------------
+ Nested Loop Left Join
+   Join Filter: (ANY (t1.a = (SubPlan 1).col1))
+   ->  Bitmap Heap Scan on rescan_bhs t1
+         ->  Bitmap Index Scan on rescan_bhs_a_idx
+   ->  Materialize
+         ->  Bitmap Heap Scan on rescan_bhs t2
+               ->  Bitmap Index Scan on rescan_bhs_a_idx
+   SubPlan 1
+     ->  Result
+           One-Time Filter: (t2.a > 1)
+           ->  Bitmap Heap Scan on rescan_bhs t3
+                 ->  Bitmap Index Scan on rescan_bhs_a_idx
+(12 rows)
+
+SELECT * FROM rescan_bhs t1 LEFT JOIN rescan_bhs t2 ON t1.a IN
+  (SELECT a FROM rescan_bhs t3 WHERE t2.a > 1);
+ a | a 
+---+---
+ 1 | 2
+ 2 | 2
+(2 rows)
+
+RESET enable_seqscan;
+RESET enable_indexscan;
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
@@ -3016,3 +3016,20 @@ SELECT t1.a FROM skip_fetch t1 LEFT JOIN skip_fetch t2 ON t2.a = 1 WHERE t2.a IS
 
 RESET enable_indexonlyscan;
 RESET enable_seqscan;
+
+-- Test BitmapHeapScan with a rescan releases resources correctly
+SET enable_seqscan = off;
+SET enable_indexscan = off;
+
+CREATE TEMP TABLE rescan_bhs (a INT);
+INSERT INTO rescan_bhs VALUES (1), (2);
+CREATE INDEX ON rescan_bhs (a);
+
+EXPLAIN (COSTS OFF)
+SELECT * FROM rescan_bhs t1 LEFT JOIN rescan_bhs t2 ON t1.a IN
+  (SELECT a FROM rescan_bhs t3 WHERE t2.a > 1);
+SELECT * FROM rescan_bhs t1 LEFT JOIN rescan_bhs t2 ON t1.a IN
+  (SELECT a FROM rescan_bhs t3 WHERE t2.a > 1);
+
+RESET enable_seqscan;
+RESET enable_indexscan;

Original file line number	Diff line number	Diff line change
`@@ -1594,7 +1594,7 @@ tbm_begin_iterate(TIDBitmap tbm, dsa_area dsa, dsa_pointer dsp)`
`1594`	`1594`	`void`
`1595`	`1595`	`tbm_end_iterate(TBMIterator *iterator)`
`1596`	`1596`	`{`
`1597`		`- Assert(iterator);`
	`1597`	`+ Assert(iterator && !tbm_exhausted(iterator));`
`1598`	`1598`
`1599`	`1599`	`if (iterator->shared)`
`1600`	`1600`	`tbm_end_shared_iterate(iterator->i.shared_iterator);`