postgres.git / summary / log / commit / refs

commit    2a527baa33372db723ca6b1b49b14da1e7a1b92e
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Wed May 07 18:25:28 2014 +0000

    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.


 src/backend/tcop/pquery.c             | 14 ++++++++++++--
 src/test/regress/expected/portals.out | 24 ++++++++++++++++++++++++
 src/test/regress/sql/portals.sql      | 11 +++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 7129e7f4f2a..ef03446a683 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -1623,6 +1623,9 @@ DoPortalRunFetch(Portal portal,
 static void
 DoPortalRewind(Portal portal)
 {
+	QueryDesc  *queryDesc;
+
+	/* Rewind holdStore, if we have one */
 	if (portal->holdStore)
 	{
 		MemoryContext oldcontext;
@@ -1631,8 +1634,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 be7348d6b2a..990bfac35d6 100644
--- a/src/test/regress/expected/portals.out
+++ b/src/test/regress/expected/portals.out
@@ -1257,3 +1257,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 585a7c25ea7..2bdad4b59fc 100644
--- a/src/test/regress/sql/portals.sql
+++ b/src/test/regress/sql/portals.sql
@@ -470,3 +470,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;


[parent: bd4c76a3c5d1]