Re: problem with RETURNING and update row movement

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: problem with RETURNING and update row movement
Date: 2020-07-22 06:16:02
Message-ID: CA+HiwqGozmgZFq2v8_gZ0n0702maZ4qh=HQusHQSx8zN8cdXwg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Amit,

Thanks for taking a look at this.

On Mon, Jul 20, 2020 at 8:35 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> IIUC, here the problem is related to below part of code:
> ExecInsert(..)
> {
> /* Process RETURNING if present */
> if (resultRelInfo->ri_projectReturning)
> result = ExecProcessReturning(resultRelInfo, slot, planSlot);
> ..
> }
>
> The reason is that planSlot is for foo1 and slot is for foo2 and when
> it tries to access tuple during ExecProcessReturning(), it results in
> an error. Is my understanding correct?

Yes. Specifically, the problem exists if there are any non-target
relation attributes in RETURNING which are computed by referring to
planSlot, the plan's output tuple, which may be shaped differently
among result relations due to their tuple descriptors being different.

> If so, then can't we ensure
> someway that planSlot also belongs to foo2 instead of skipping return
> processing in Insert and then later do more work to perform in Update.

I did consider that option but failed to see a way to make it work.

I am not sure if there is a way to make a copy of the plan's output
tuple (planSlot) that is compatible with the destination partition.
Simple conversion using execute_attr_map_slot() is okay when we know
the source and the target slots contain relation tuples, but plan's
output tuples will also have other junk attributes. Also, not all
destination partitions have an associated plan and hence a slot to
hold plan tuples.

> Like Takamichi-san, I also think here we don't need trigger/function
> in the test case. If one reads the comment you have added in the test
> case, it is not evident why is trigger or function required. If you
> really think it is important to cover the trigger case then either
> have a separate test or at least add some comments on how trigger
> helps here or what you want to test it.

That's fair. I have updated the test comment.

To expand on that here, because now we'll be computing RETURNING using
the source partition's projection and the tuple in the source
partition's format, I wanted to make sure that any changes made by the
destination partition's triggers are reflected in the output.

PFA v2.

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

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message k.jamison@fujitsu.com 2020-07-22 06:17:33 RE: Parallel Seq Scan vs kernel read ahead
Previous Message osumi.takamichi@fujitsu.com 2020-07-22 05:41:15 RE: Implement UNLOGGED clause for COPY FROM