Skip to content

Commit 022b5f2

Browse files
tglsfdctglsfdc
committed
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.
1 parent dfac804 commit 022b5f2

File tree

3 files changed

+47
-2
lines changed

3 files changed

+47
-2
lines changed

‎src/backend/tcop/pquery.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1661,6 +1661,9 @@ DoPortalRunFetch(Portal portal,
16611661
static void
16621662
DoPortalRewind(Portal portal)
16631663
{
1664+
QueryDesc *queryDesc;
1665+
1666+
/* Rewind holdStore, if we have one */
16641667
if (portal->holdStore)
16651668
{
16661669
MemoryContext oldcontext;
@@ -1669,8 +1672,15 @@ DoPortalRewind(Portal portal)
16691672
tuplestore_rescan(portal->holdStore);
16701673
MemoryContextSwitchTo(oldcontext);
16711674
}
1672-
if (PortalGetQueryDesc(portal))
1673-
ExecutorRewind(PortalGetQueryDesc(portal));
1675+
1676+
/* Rewind executor, if active */
1677+
queryDesc = PortalGetQueryDesc(portal);
1678+
if (queryDesc)
1679+
{
1680+
PushActiveSnapshot(queryDesc->snapshot);
1681+
ExecutorRewind(queryDesc);
1682+
PopActiveSnapshot();
1683+
}
16741684

16751685
portal->atStart = true;
16761686
portal->atEnd = false;

‎src/test/regress/expected/portals.out

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,3 +1257,27 @@ FETCH ALL FROM c1;
12571257

12581258
COMMIT;
12591259
DROP TABLE cursor;
1260+
-- Check rewinding a cursor containing a stable function in LIMIT,
1261+
-- per bug report in 8336843.9833.1399385291498.JavaMail.root@quick
1262+
begin;
1263+
create function nochange(int) returns int
1264+
as 'select $1 limit 1' language sql stable;
1265+
declare c cursor for select * from int8_tbl limit nochange(3);
1266+
fetch all from c;
1267+
q1 | q2
1268+
------------------+------------------
1269+
123 | 456
1270+
123 | 4567890123456789
1271+
4567890123456789 | 123
1272+
(3 rows)
1273+
1274+
move backward all in c;
1275+
fetch all from c;
1276+
q1 | q2
1277+
------------------+------------------
1278+
123 | 456
1279+
123 | 4567890123456789
1280+
4567890123456789 | 123
1281+
(3 rows)
1282+
1283+
rollback;

‎src/test/regress/sql/portals.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,3 +470,14 @@ UPDATE cursor SET a = 2;
470470
FETCH ALL FROM c1;
471471
COMMIT;
472472
DROP TABLE cursor;
473+
474+
-- Check rewinding a cursor containing a stable function in LIMIT,
475+
-- per bug report in 8336843.9833.1399385291498.JavaMail.root@quick
476+
begin;
477+
create function nochange(int) returns int
478+
as 'select $1 limit 1' language sql stable;
479+
declare c cursor for select * from int8_tbl limit nochange(3);
480+
fetch all from c;
481+
move backward all in c;
482+
fetch all from c;
483+
rollback;

0 commit comments

Comments
 (0)