pgsql: Fix deletion of speculatively inserted TOAST on conflict
Fix deletion of speculatively inserted TOAST on conflict
INSERT .. ON CONFLICT runs a pre-check of the possible conflicting
constraints before performing the actual speculative insertion. In case
the inserted tuple included TOASTed columns the ON CONFLICT condition
would be handled correctly in case the conflict was caught by the
pre-check, but if two transactions entered the speculative insertion
phase at the same time, one would have to re-try, and the code for
aborting a speculative insertion did not handle deleting the
speculatively inserted TOAST datums correctly.
TOAST deletion would fail with "ERROR: attempted to delete invisible
tuple" as we attempted to remove the TOAST tuples using
simple_heap_delete which reasoned that the given tuples should not be
visible to the command that wrote them.
This commit updates the heap_abort_speculative() function which aborts
the conflicting tuple to use itself, via toast_delete, for deleting
associated TOAST datums. Like before, the inserted toast rows are not
marked as being speculative.
This commit also adds a isolationtester spec test, exercising the
relevant code path. Unfortunately 9.5 cannot handle two waiting
sessions, and thus cannot execute this test.
Reported-By: Viren Negi, Oskari Saarenmaa
Author: Oskari Saarenmaa, edited a bit by me
Bug: #14150
Discussion: <20160519123338.12513.20271@wrigleys.postgresql.org>
Backpatch: 9.5, where ON CONFLICT was introduced
Branch
------
REL9_5_STABLE
Details
-------
http://git.postgresql.org/pg/commitdiff/94bc30725aed39498652e1394422d553ea305634
Modified Files
--------------
src/backend/access/heap/heapam.c | 12 ++++++++----
src/backend/access/heap/tuptoaster.c | 15 +++++++++------
src/backend/utils/time/tqual.c | 4 ++--
src/include/access/tuptoaster.h | 2 +-
4 files changed, 20 insertions(+), 13 deletions(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On Wed, Aug 17, 2016 at 8:06 PM Andres Freund <andres@anarazel.de> wrote:
This commit updates the heap_abort_speculative() function which aborts
the conflicting tuple to use itself, via toast_delete, for deleting
associated TOAST datums. Like before, the inserted toast rows are not
marked as being speculative.
I just noticed how crazy this is - not the commit itself
(07ef035129586ca26a713c4cd15e550dfe35e643) but the thing which the
commit message describes as pre-existing behavior. Apparently, even
if the insertion wasn't speculative, you can still abort it just as if
it had been, at least when we're talking about a TOAST table row. Not
that I have a better idea, but are we sure that's the way we want to
go?
This is relevant to my little project to make the TOAST logic reusable
by other AMs, because the comments in tableam.h suggest you can only
complete a speculative insertion if you've previously performed one.
If we allow any AM to be used to implement a TOAST table, then it
needs to be documented that such AMs have to cope with this kind of
case.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Hi,
On 2019-06-11 11:47:07 -0400, Robert Haas wrote:
On Wed, Aug 17, 2016 at 8:06 PM Andres Freund <andres@anarazel.de> wrote:
This commit updates the heap_abort_speculative() function which aborts
the conflicting tuple to use itself, via toast_delete, for deleting
associated TOAST datums. Like before, the inserted toast rows are not
marked as being speculative.I just noticed how crazy this is - not the commit itself
(07ef035129586ca26a713c4cd15e550dfe35e643) but the thing which the
commit message describes as pre-existing behavior. Apparently, even
if the insertion wasn't speculative, you can still abort it just as if
it had been, at least when we're talking about a TOAST table row. Not
that I have a better idea, but are we sure that's the way we want to
go?
Well, less "we want to go", and more "have been going for ~five
years"...
I don't think I immediately see - or saw back then - a realistic better
approach. I guess we could add a flag that somehow signals that a plain
delete should accept an invisible tuple as input, but that seems
dangerous too.
This is relevant to my little project to make the TOAST logic reusable
by other AMs, because the comments in tableam.h suggest you can only
complete a speculative insertion if you've previously performed one.
If we allow any AM to be used to implement a TOAST table, then it
needs to be documented that such AMs have to cope with this kind of
case.
Hm - you're thinking of making the case of toast AM and main AM being
different working? I'm not sure I'd otherwise expect to again go through
the AM, although I'm not sure about that.
Greetings,
Andres Freund
On Tue, Jun 11, 2019 at 12:06 PM Andres Freund <andres@anarazel.de> wrote:
This is relevant to my little project to make the TOAST logic reusable
by other AMs, because the comments in tableam.h suggest you can only
complete a speculative insertion if you've previously performed one.
If we allow any AM to be used to implement a TOAST table, then it
needs to be documented that such AMs have to cope with this kind of
case.Hm - you're thinking of making the case of toast AM and main AM being
different working? I'm not sure I'd otherwise expect to again go through
the AM, although I'm not sure about that.
I was, but I think we have the same requirement even if we don't,
because detoasting a datum from anywhere goes through a common code
path.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company