From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: a.wicht@gmail.com, pgsql-bugs@lists.postgresql.org, Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Subject: Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries
Date: Tue, 11 May 2021 23:25:57 -0400
Message-ID: <1032535.1620789957@sss.pgh.pa.us> (raw)
In-Reply-To: <957186.1620784871@sss.pgh.pa.us>
References: <15990-eee2ac466b11293d@postgresql.org>
<20190904105618.j5l6fhyesmprmstf@alap3.anarazel.de>
<912290.1620774094@sss.pgh.pa.us>
<957186.1620784871@sss.pgh.pa.us>
I wrote:
> I think the bigger-picture question is, if we're trying to detoast
> as the first step in a new transaction of a procedure, where's the
> guarantee that the TOAST data still exists to be fetched? For sure
> we aren't holding any locks that would stop VACUUM from reclaiming
> recently-dead TOAST rows.
Yeah. So here's a patch I actually believe in to some extent, based
on Konstantin's idea.
> I'm still wondering why plpgsql-toast.spec is failing to show the
> problem, too.
The answer to that is that it's not testing commit-inside-a-cursor-loop,
and if it were, it'd still fail to show the problem because its test
table contains just one row.
Interestingly, if you try the test case added here without adding the
code patch, you get a "missing chunk number ... for toast value ..."
error, not "no known snapshots". I think that's because the test
case has additional commands after the COMMIT, causing the transaction
snapshot to get re-established.
regards, tom lane
Attachments:
[text/x-diff] fix-commit-inside-cursor-loop.patch (3.1K, ../1032535.1620789957@sss.pgh.pa.us/2-fix-commit-inside-cursor-loop.patch)
download | inline diff:diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.cindex b4c70aaa7f..27d6ea7551 100644--- a/src/pl/plpgsql/src/pl_exec.c+++ b/src/pl/plpgsql/src/pl_exec.c@@ -5826,6 +5826,17 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt,
*/
PinPortal(portal);
+ /*+ * In a non-atomic context, we dare not prefetch, even if it would+ * otherwise be safe. Aside from any semantic hazards that that might+ * create, if we prefetch toasted data and then the user commits the+ * transaction, the toast references could turn into dangling pointers.+ * (Rows we haven't yet fetched from the cursor are safe, because the+ * PersistHoldablePortal mechanism handles this scenario.)+ */+ if (!estate->atomic)+ prefetch_ok = false;+
/*
* Fetch the initial tuple(s). If prefetching is allowed then we grab a
* few more rows to avoid multiple trips through executor startup
diff --git a/src/test/isolation/expected/plpgsql-toast.out b/src/test/isolation/expected/plpgsql-toast.outindex fc557da5e7..4f216b94b6 100644--- a/src/test/isolation/expected/plpgsql-toast.out+++ b/src/test/isolation/expected/plpgsql-toast.out@@ -192,3 +192,46 @@ pg_advisory_unlock
t
s1: NOTICE: length(r) = 6002
step assign5: <... completed>
++starting permutation: lock assign6 vacuum unlock+pg_advisory_unlock_all++ +pg_advisory_unlock_all++ +step lock: + SELECT pg_advisory_lock(1);++pg_advisory_lock++ +step assign6: +do $$+ declare+ r record;+ begin+ insert into test1 values (2, repeat('bar', 3000));+ insert into test1 values (3, repeat('baz', 4000));+ for r in select test1.b from test1 loop+ delete from test1;+ commit;+ perform pg_advisory_lock(1);+ raise notice 'length(r) = %', length(r::text);+ end loop;+ end;+$$;+ <waiting ...>+step vacuum: + VACUUM test1;++step unlock: + SELECT pg_advisory_unlock(1);++pg_advisory_unlock++t +s1: NOTICE: length(r) = 6002+s1: NOTICE: length(r) = 9002+s1: NOTICE: length(r) = 12002+step assign6: <... completed>diff --git a/src/test/isolation/specs/plpgsql-toast.spec b/src/test/isolation/specs/plpgsql-toast.specindex fe7090addb..bd2948b3d3 100644--- a/src/test/isolation/specs/plpgsql-toast.spec+++ b/src/test/isolation/specs/plpgsql-toast.spec@@ -112,6 +112,25 @@ do $$
$$;
}
+# committing inside a cursor loop presents its own hazards+step "assign6"+{+do $$+ declare+ r record;+ begin+ insert into test1 values (2, repeat('bar', 3000));+ insert into test1 values (3, repeat('baz', 4000));+ for r in select test1.b from test1 loop+ delete from test1;+ commit;+ perform pg_advisory_lock(1);+ raise notice 'length(r) = %', length(r::text);+ end loop;+ end;+$$;+}+
session "s2"
setup
{
@@ -135,3 +154,4 @@ permutation "lock" "assign2" "vacuum" "unlock"
permutation "lock" "assign3" "vacuum" "unlock"
permutation "lock" "assign4" "vacuum" "unlock"
permutation "lock" "assign5" "vacuum" "unlock"
+permutation "lock" "assign6" "vacuum" "unlock"
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, robertmhaas@gmail.com, andres@anarazel.de, peter.eisentraut@enterprisedb.com
Subject: Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries
In-Reply-To: <1032535.1620789957@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox