From 669e72feb29262110fb7085601d1042b781d3a2b Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi Date: Tue, 29 Mar 2022 17:53:39 +0900 Subject: [PATCH v1] Appropriately set snapshot on cursor fetch 84f5c2908d thought that FETCH doesn't need a snapshot, but it is actually needed. FETCH can crash when accessing a toast value. To fix this, teach PlannedStmtRequiresSnapshot that FETCH needs a snapshot then tell RunFromStore to use the snapshot. Reported-by: Erik Rijkers --- src/backend/tcop/pquery.c | 17 ++++++++++++++++- src/test/regress/expected/portals.out | 20 ++++++++++++++++++++ src/test/regress/sql/portals.sql | 22 ++++++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index 5aa5a350f3..08b37fb63d 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -1068,6 +1068,15 @@ RunFromStore(Portal portal, ScanDirection direction, uint64 count, dest->rStartup(dest, CMD_SELECT, portal->tupDesc); + /* Valid holdSnapshot here means it should be used to read this store. */ + if (portal->holdSnapshot) + { + Assert(portal->portalSnapshot == 0); + PushActiveSnapshotWithLevel(portal->holdSnapshot, + portal->createLevel); + portal->portalSnapshot = GetActiveSnapshot(); + } + if (ScanDirectionIsNoMovement(direction)) { /* do nothing except start/stop the destination */ @@ -1114,6 +1123,13 @@ RunFromStore(Portal portal, ScanDirection direction, uint64 count, dest->rShutdown(dest); + if (portal->holdSnapshot) + { + Assert(portal->portalSnapshot == GetActiveSnapshot()); + PopActiveSnapshot(); + portal->portalSnapshot = NULL; + } + ExecDropSingleTupleTableSlot(slot); return current_tuple_count; @@ -1756,7 +1772,6 @@ PlannedStmtRequiresSnapshot(PlannedStmt *pstmt) IsA(utilityStmt, VariableShowStmt) || IsA(utilityStmt, ConstraintsSetStmt) || /* efficiency hacks from here down */ - IsA(utilityStmt, FetchStmt) || IsA(utilityStmt, ListenStmt) || IsA(utilityStmt, NotifyStmt) || IsA(utilityStmt, UnlistenStmt) || diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out index 9da74876e1..fd56859370 100644 --- a/src/test/regress/expected/portals.out +++ b/src/test/regress/expected/portals.out @@ -1536,3 +1536,23 @@ fetch backward all in c2; (3 rows) rollback; +-- Check if snapshot is available for cursor while accessing toast value +begin; +create function r (l int) returns text as $$ +DECLARE + i int; + s text; +BEGIN + s := ''; + for i in 0..l loop + s := s || chr((random() * 254)::int + 1); + end loop; + return s; +END; +$$ language plpgsql; +create table t(a text); +insert into t values(r(4192)); +declare c cursor for select a from t; +-- the following shouldn't crash +fetch all in c \gset result +rollback; diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql index eadf6ed942..0dfe0ad981 100644 --- a/src/test/regress/sql/portals.sql +++ b/src/test/regress/sql/portals.sql @@ -581,3 +581,25 @@ declare c2 scroll cursor for select generate_series(1,3) as g; fetch all in c2; fetch backward all in c2; rollback; + +-- Check if snapshot is available for cursor while accessing toast value +begin; +create function r (l int) returns text as $$ +DECLARE + i int; + s text; +BEGIN + s := ''; + for i in 0..l loop + s := s || chr((random() * 254)::int + 1); + end loop; + return s; +END; +$$ language plpgsql; + +create table t(a text); +insert into t values(r(4192)); +declare c cursor for select a from t; +-- the following shouldn't crash +fetch all in c \gset result +rollback; -- 2.27.0