Re: BitmapHeapScan streaming read user and prelim refactoring - Mailing list pgsql-hackers

On Sat, Oct 19, 2024 at 5:48 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> I plan to commit 0002 and 0003 next week. I'm interested if you think
> 0001 is correct.
> I may also commit 0004-0006 as I feel they are ready too.

I noticed an oversight on master, which I think was introduced by the
0005 patch.  In ExecReScanBitmapHeapScan:

-      if (scan->st.bitmap.rs_shared_iterator)
-      {
-              tbm_end_shared_iterate(scan->st.bitmap.rs_shared_iterator);
-              scan->st.bitmap.rs_shared_iterator = NULL;
-      }
-
-      if (scan->st.bitmap.rs_iterator)
-      {
-              tbm_end_private_iterate(scan->st.bitmap.rs_iterator);
-              scan->st.bitmap.rs_iterator = NULL;
-      }
+      tbm_end_iterate(&scan->st.rs_tbmiterator);

I don't think it's safe to call tbm_end_iterate directly on
scan->st.rs_tbmiterator, as it may have already been cleaned up by
this point.

Here is a query that triggers a SIGSEGV fault on master.

create table t (a int);
insert into t values (1), (2);
create index on t (a);

set enable_seqscan to off;
set enable_indexscan to off;

select * from t t1 left join t t2 on t1.a in (select a from t t3 where
t2.a > 1);


I think we need to check whether rs_tbmiterator is NULL before calling
tbm_end_iterate on it, like below.

--- 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
+        * haven't 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);

Thanks
Richard