8000 Fix failure to set ActiveSnapshot while rewinding a cursor. · postgrespro/postgres_cluster@04e5025 · GitHub
[go: up one dir, main page]
Skip to content

Commit 04e5025

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 e2ce9aa commit 04e5025

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
@@ -1261,3 +1261,27 @@ FETCH ALL FROM c1;
12611261

12621262
COMMIT;
12631263
DROP TABLE cursor;
1264+
-- Check rewinding a cursor containing a stable function in LIMIT,
1265+
-- per bug report in 8336843.9833.1399385291498.JavaMail.root@quick
1266+
begin;
1267+
create function nochange(int) returns int
1268+
as 'select $1 limit 1' language sql stable;
1269+
declare c cursor for select * from int8_tbl limit nochange(3);
1270+
fetch all from c;
1271+
q1 | q2
1272+
------------------+------------------
1273+
123 | 456
1274+
123 | 4567890123456789
1275+
4567890123456789 | 123
1276+
(3 rows)
1277+
1278+
move backward all in c;
1279+
fetch all from c;
1280+
q1 | q2
1281< 8000 span class="diff-text-marker">+
------------------+------------------
1282+
123 | 456
1283+
123 | 4567890123456789
1284+
4567890123456789 | 123
1285+
(3 rows)
1286+
1287+
rollback;

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

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

0 commit comments

Comments
 (0)
0