pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
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>
Cc: Konstantin Knizhnik <k.knizhnik@postgrespro.ru>
Subject: Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries
Date: Tue, 11 May 2021 22:01:11 -0400
Message-ID: <957186.1620784871@sss.pgh.pa.us> (raw)
In-Reply-To: <912290.1620774094@sss.pgh.pa.us>
References: <15990-eee2ac466b11293d@postgresql.org>
	<20190904105618.j5l6fhyesmprmstf@alap3.anarazel.de>
	<912290.1620774094@sss.pgh.pa.us>

[ Roping Robert into this, as committer of 3e2f3c2e4 ]

I wrote:
> After an admittedly cursory look-around, it seems like the problem
> can be stated as "init_toast_snapshot expects that there already
> is a transaction snapshot, which there is not because we just
> committed and nothing has re-established a transaction snapshot".
> So the question is, where shall we force a new transaction snapshot
> to be created after a COMMIT/ROLLBACK inside a procedure?

> The most localized fix would be to let init_toast_snapshot itself
> do that, but that seems like a bit of a layering violation; plus
> I'm not quite convinced that's the only place with the issue.

I tried this, which leads to a nicely small patch and seems to resolve
the existing reports, but now I'm not sure that it's actually safe.
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.

In a recent discussion at [1], Konstantin Knizhnik reasoned that the
problem is that plpgsql is holding rows that it's prefetched but not
yet detoasted, and proposed disabling prefetch to solve this.  I think
he's probably right, although his patch strikes me as both overcomplicated
and wrong.  I suspect we must disable prefetch in any non-atomic
execution context, because we can't know whether a COMMIT will be executed
by some called procedure.

I'm still wondering why plpgsql-toast.spec is failing to show the
problem, too.

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/03644c0e6bb82132ac783982b6abffdf%40postgrespro.ru

Attachments:

  [text/x-diff] probably-incorrect-toast-fix.patch (695B, ../957186.1620784871@sss.pgh.pa.us/2-probably-incorrect-toast-fix.patch)
  download | inline diff:
diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c
index 730cd04a2d..386c5bda2b 100644
--- a/src/backend/access/common/toast_internals.c
+++ b/src/backend/access/common/toast_internals.c
@@ -638,8 +638,12 @@ init_toast_snapshot(Snapshot toast_snapshot)
 {
 	Snapshot	snapshot = GetOldestSnapshot();
 
+	/*
+	 * It is possible to get here when no snapshot has yet been established in
+	 * the current transaction.  If so, just create a transaction snapshot.
+	 */
 	if (snapshot == NULL)
-		elog(ERROR, "no known snapshots");
+		snapshot = GetTransactionSnapshot();
 
 	InitToastSnapshot(*toast_snapshot, snapshot->lsn, snapshot->whenTaken);
 }

view thread (10+ messages)  latest in thread

Message-ID: <957186.1620784871@sss.pgh.pa.us>
Permalink:  ../957186.1620784871@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/957186.1620784871@sss.pgh.pa.us

 ·  · 

reply

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, k.knizhnik@postgrespro.ru
  Subject: Re: BUG #15990: PROCEDURE throws "SQL Error [XX000]: ERROR: no known snapshots" with PostGIS geometries
  In-Reply-To: <957186.1620784871@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