Re: BUG #15378: SP-GIST memory context screwup?

>>>>> "Andrew" == Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

 Andrew> So the simplest fix would be to move the memory context reset
 Andrew> to just after the freeScanStack in resetSpGistScanOpaque. And
 Andrew> with retail freeing going on during the scan, it makes less
 Andrew> sense to try and avoid it during rescan, double ugh.

Proposed patch attached.

-- 
Andrew (irc:RhodiumToad)

diff --git a/src/backend/access/spgist/spgscan.c b/src/backend/access/spgist/spgscan.c
index c728080812..5260d5017d 100644
--- a/src/backend/access/spgist/spgscan.c
+++ b/src/backend/access/spgist/spgscan.c
@@ -74,6 +74,13 @@ resetSpGistScanOpaque(SpGistScanOpaque so)
 
     freeScanStack(so);
 
+    /*
+     * clear traversal context before proceeding to the next scan; this must
+     * not happen before the freeScanStack above, else we get double-free
+     * crashes.
+     */
+    MemoryContextReset(so->traversalCxt);
+
     if (so->searchNulls)
     {
         /* Stack a work item to scan the null index entries */
@@ -212,9 +219,6 @@ spgrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
 {
     SpGistScanOpaque so = (SpGistScanOpaque) scan->opaque;
 
-    /* clear traversal context before proceeding to the next scan */
-    MemoryContextReset(so->traversalCxt);
-
     /* copy scankeys into local storage */
     if (scankey && scan->numberOfKeys > 0)
     {
diff --git a/src/test/regress/expected/spgist.out b/src/test/regress/expected/spgist.out
index 2d75bbf8dc..9364b88bc2 100644
--- a/src/test/regress/expected/spgist.out
+++ b/src/test/regress/expected/spgist.out
@@ -23,6 +23,24 @@ delete from spgist_point_tbl where id % 2 = 1;
 -- would exercise it)
 delete from spgist_point_tbl where id < 10000;
 vacuum spgist_point_tbl;
+-- Test rescan paths (cf. bug #15378)
+-- use box and && rather than point, so that rescan happens when the
+-- traverse stack is non-empty
+create table spgist_box_tbl(id serial, b box);
+insert into spgist_box_tbl(b)
+select box(point(i,j),point(i+s,j+s))
+  from generate_series(1,100,5) i,
+       generate_series(1,100,5) j,
+       generate_series(1,10) s;
+create index spgist_box_idx on spgist_box_tbl using spgist (b);
+select count(*)
+  from (values (point(5,5)),(point(8,8)),(point(12,12))) v(p)
+ where exists(select * from spgist_box_tbl b where b.b && box(v.p,v.p));
+ count 
+-------
+     3
+(1 row)
+
 -- The point opclass's choose method only uses the spgMatchNode action,
 -- so the other actions are not tested by the above. Create an index using
 -- text opclass, which uses the others actions.
diff --git a/src/test/regress/sql/spgist.sql b/src/test/regress/sql/spgist.sql
index 77b43a2d3e..c72cf42a33 100644
--- a/src/test/regress/sql/spgist.sql
+++ b/src/test/regress/sql/spgist.sql
@@ -30,6 +30,21 @@ delete from spgist_point_tbl where id < 10000;
 
 vacuum spgist_point_tbl;
 
+-- Test rescan paths (cf. bug #15378)
+-- use box and && rather than point, so that rescan happens when the
+-- traverse stack is non-empty
+
+create table spgist_box_tbl(id serial, b box);
+insert into spgist_box_tbl(b)
+select box(point(i,j),point(i+s,j+s))
+  from generate_series(1,100,5) i,
+       generate_series(1,100,5) j,
+       generate_series(1,10) s;
+create index spgist_box_idx on spgist_box_tbl using spgist (b);
+
+select count(*)
+  from (values (point(5,5)),(point(8,8)),(point(12,12))) v(p)
+ where exists(select * from spgist_box_tbl b where b.b && box(v.p,v.p));
 
 -- The point opclass's choose method only uses the spgMatchNode action,
 -- so the other actions are not tested by the above. Create an index using