Re: problem with RETURNING and update row movement

On Thu, Jun 11, 2020 at 2:40 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> Hi,
>
> While working on [1], I came across a bug.
>
> Reproduction steps:
>
> create table foo (a int, b int) partition by list (a);
> create table foo1 (c int, b int, a int);
> alter table foo1 drop c;
> alter table foo attach partition foo1 for values in (1);
> create table foo2 partition of foo for values in (2);
> create table foo3 partition of foo for values in (3);
> create or replace function trigfunc () returns trigger language
> plpgsql as $$ begin new.b := 2; return new; end; $$;
> create trigger trig before insert on foo2 for each row execute
> function trigfunc();
> insert into foo values (1, 1), (2, 2), (3, 3);
> update foo set a = 2 from (values (1), (2), (3)) s(x) where a = s.x returning *;
> ERROR: attribute 5 of type record has wrong type
> DETAIL: Table has type record, but query expects integer.
>
> The error occurs when projecting the RETURNING list. The problem is
> that the projection being used when the error occurs belongs to result
> relation foo2 which is the destination partition of a row movement
> operation, but it's trying to access a column in the tuple produced by
> the plan belonging to foo1, the source partition of the row movement.
> foo2's RETURNING projection can only work correctly when it's being
> fed tuples from the plan belonging to foo2.
>

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? 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.

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.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com