At Tue, 29 Mar 2022 17:06:21 +0900 (JST), Kyotaro Horiguchi
<horikyota....@gmail.com> wrote in
> At Mon, 28 Mar 2022 18:36:46 +0900 (JST), Kyotaro Horiguchi
> <horikyota....@gmail.com> wrote in
> Then, I found that portal->holdSnapshot is that. I came up with the
> attached. It does the follows:
>
> 1. Teach PlannedStmtRequiresSnapshot() to return true for FetchStmt.
>
> 2. Use holdSnapshot in RunFromStore if any.
>
>
> The rerpducer is reduced to as small as the following.
>
> CREATE TABLE t (a text);
> INSERT INTO t VALUES('some random text');
> BEGIN;
> DECLARE c CURSOR FOR SELECT * FROM t;
> FETCH ALL IN c;
>
> But I haven't come up with a reasonable way to generate the 'some
> random text' yet.
I gave up and took a straightforward way to generate one.
I don't like that it uses a fixed length for the random text, but
anyway it works for now...
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 669e72feb29262110fb7085601d1042b781d3a2b Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota....@gmail.com>
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 <e...@xs4all.nl>
---
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
