Re: problem with RETURNING and update row movement

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: problem with RETURNING and update row movement
Date: 2020-10-30 10:00:42
Message-ID: CA+HiwqE=X6qBC_Eo2JxfEz9VVTW6GcefyBg592bzcUVgCPwFZg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 30, 2020 at 12:34 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> On Thu, Sep 24, 2020 at 2:26 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > I saw the CF-bot failure too yesterday, although it seems that it's
> > because the bot is trying to apply the patch version meant for v11
> > branch onto HEAD branch. I just checked that the patches apply
> > cleanly to their respective branches.
>
> I checked that the last statement is still true, so I've switched the
> status back to Needs Review.

And the patch for HEAD no longer applies, because of a recent
refactoring commit that hit update row movement code.

Rebased patch for HEAD attached. Patch for PG11 should apply as-is.

Here is a summary of where we stand on this, because another issue
related to using RETURNING with partitioned tables [1] kind of ties
into this.

The problem reported on this thread is solved by ExecUpdate() itself
evaluating the RETURNING list using the source partition's
ri_projectReturning, instead of ExecInsert() doing it using the
destination partition's ri_projectReturning. It must work that way,
because the tuple descriptor of 'planSlot', which provides the values
of the columns mentioned in RETURNING of tables other than the target
table, is based on the source partition. Note that the tuple inserted
into the destination partition needs to be converted into the source
partition if their tuple descriptors differ, so that RETURNING
correctly returns the new values of the updated tuple.

The point we are stuck on is whether this fix will create problems for
the case when the RETURNING list contains system columns. Now that we
will be projecting the tuple inserted into the destination using its
copy in the source partition's format and we have no way of preserving
the system information in the tuple across conversions, we need to
find a way to make RETURNING project the correct system information.
Fujita-san has suggested (or agreed with a suggestion made at [1])
that we should simply prevent system information from being projected
with RETURNING when the command is being performed on a partitioned
table, in which case this becomes a non-issue. If that is not what we
decide to do on the other thread, then at least we will have to figure
out over there how we will support RETURNING system columns when row
movement occurs during an update on partitioned tables. The question
I guess is whether that thread must conclude before the fix here can
be committed.

--
Amit Langote
EDB: http://www.enterprisedb.com

[1] https://www.postgresql.org/message-id/flat/141051591267657%40mail.yandex.ru

Attachment Content-Type Size
v6-0001-Fix-a-bug-with-RETURNING-when-UPDATE-moves-tuple-HEAD.patch application/octet-stream 12.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2020-10-30 10:51:44 Re: [HACKERS] logical decoding of two-phase transactions
Previous Message Amit Kapila 2020-10-30 09:30:20 Re: Enumize logical replication message actions