Fix failure to set ActiveSnapshot while rewinding a cursor.

ActiveSnapshot needs to be set when we call ExecutorRewind because some
plan node types may execute user-defined functions during their ReScan
calls (nodeLimit.c does so, at least).  The wisdom of that is somewhat
debatable, perhaps, but for now the simplest fix is to make sure the
required context is valid.  Failure to do this typically led to a
null-pointer-dereference core dump, though it's possible that in more
complex cases a function could be executed with the wrong snapshot
leading to very subtle misbehavior.

Per report from Leif Jensen.  It's been broken for a long time, so
back-patch to all active branches.

diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index fa561a4861a3fdabb7246a23af654d7e8a5dd9a1..f9ed266c1ad6f65ea5ecd0e7e882cd3144866620 100644 (file)
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -1661,6 +1661,9 @@ DoPortalRunFetch(Portal portal,
 static void
 DoPortalRewind(Portal portal)
 {
+       QueryDesc  *queryDesc;
+
+       /* Rewind holdStore, if we have one */
        if (portal->holdStore)
        {
                MemoryContext oldcontext;
@@ -1669,8 +1672,15 @@ DoPortalRewind(Portal portal)
                tuplestore_rescan(portal->holdStore);
                MemoryContextSwitchTo(oldcontext);
        }
-       if (PortalGetQueryDesc(portal))
-               ExecutorRewind(PortalGetQueryDesc(portal));
+
+       /* Rewind executor, if active */
+       queryDesc = PortalGetQueryDesc(portal);
+       if (queryDesc)
+       {
+               PushActiveSnapshot(queryDesc->snapshot);
+               ExecutorRewind(queryDesc);
+               PopActiveSnapshot();
+       }
 
        portal->atStart = true;
        portal->atEnd = false;

diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out
index aafc6cf529416ae3e13c063c8f805dab1e578cdb..462ad231c368d294045bafdd4bc847493b55cbe0 100644 (file)
--- a/src/test/regress/expected/portals.out
+++ b/src/test/regress/expected/portals.out
@@ -1261,3 +1261,27 @@ FETCH ALL FROM c1;
 
 COMMIT;
 DROP TABLE cursor;
+-- Check rewinding a cursor containing a stable function in LIMIT,
+-- per bug report in 8336843.9833.1399385291498.JavaMail.root@quick
+begin;
+create function nochange(int) returns int
+  as 'select $1 limit 1' language sql stable;
+declare c cursor for select * from int8_tbl limit nochange(3);
+fetch all from c;
+        q1        |        q2        
+------------------+------------------
+              123 |              456
+              123 | 4567890123456789
+ 4567890123456789 |              123
+(3 rows)
+
+move backward all in c;
+fetch all from c;
+        q1        |        q2        
+------------------+------------------
+              123 |              456
+              123 | 4567890123456789
+ 4567890123456789 |              123
+(3 rows)
+
+rollback;

diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql
index 203e6577039201c317ea223ef867ba1b781e94aa..01c3b85da9a763697fd4e5eac3f5984f24e88c58 100644 (file)
--- a/src/test/regress/sql/portals.sql
+++ b/src/test/regress/sql/portals.sql
@@ -473,3 +473,14 @@ UPDATE cursor SET a = 2;
 FETCH ALL FROM c1;
 COMMIT;
 DROP TABLE cursor;
+
+-- Check rewinding a cursor containing a stable function in LIMIT,
+-- per bug report in 8336843.9833.1399385291498.JavaMail.root@quick
+begin;
+create function nochange(int) returns int
+  as 'select $1 limit 1' language sql stable;
+declare c cursor for select * from int8_tbl limit nochange(3);
+fetch all from c;
+move backward all in c;
+fetch all from c;
+rollback;