8000 Fix failure to set ActiveSnapshot while rewinding a cursor. · lansz/postgres@2a527ba · GitHub
[go: up one dir, main page]
Skip to content

Commit 2a527ba

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 bd4c76a commit 2a527ba

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
@@ -1623,6 +1623,9 @@ DoPortalRunFetch(Portal portal,
16231623
static void
16241624
DoPortalRewind(Portal portal)
16251625
{
1626+
QueryDesc *queryDesc;
1627+
1628+
/* Rewind holdStore, if we have one */
16261629
if (portal->holdStore)
16271630
{
16281631
MemoryContext oldcontext;
@@ -1631,8 +1634,15 @@ DoPortalRewind(Portal portal)
16311634
tuplestore_rescan(portal->holdStore);
16321635
MemoryContextSwitchTo(oldcontext);
16331636
}
1634-
if (PortalGetQueryDesc(portal))
1635-
ExecutorRewind(PortalGetQueryDesc(portal));
1637+
1638+
/* Rewind executor, if active */
1639+
queryDesc = PortalGetQueryDesc(portal);
1640+
if (queryDesc)
1641+
{
1642+
PushActiveSnapshot(queryDesc->snapshot);
1643+
ExecutorRewind(queryDesc);
1644+
PopActiveSnapshot();
1645+
}
16361646

16371647
portal->atStart = true;
16381648
portal->atEnd = false;

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

Lines changed: 24 additions & 0 deletions
< 8000 /div>
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)
0