Re: Bug in ExecModifyTable function and trigger issues for foreign tables

On Mon, May 15, 2017 at 7:24 PM, Ildus Kurbangaliev
<i(dot)kurbangaliev(at)postgrespro(dot)ru> wrote:
> On Mon, 15 May 2017 17:43:52 +0530
> Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>
>> On Mon, May 15, 2017 at 2:46 PM, Ildus Kurbangaliev
>> <i(dot)kurbangaliev(at)postgrespro(dot)ru> wrote:
>> > On Mon, 15 May 2017 10:34:58 +0530
>> > Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>> >
>> >> On Sun, May 14, 2017 at 9:54 PM, Dilip Kumar
>> >> <dilipbalaut(at)gmail(dot)com> wrote:
>> >> > After your fix, now tupleid is invalid which is expected, but
>> >> > seems like we need to do something more. As per the comments
>> >> > seems like it is expected to get the oldtuple from planSlot.
>> >> > But I don't see any code for handling that part.
>> >>
>> >> Maybe we should do something like attached patch.
>> >>
>> >
>> > Hi,
>> > planSlot contains already projected tuple, you can't use it as
>> > oldtuple. I think problem is that `rewriteTargetListUD` called only
>> > for parent relation, so there is no `wholerow` attribute for
>> > foreign tables.
>>
>> Yes. postgresAddForeignUpdateTargets() which is called by
>> rewriteTargetListUD injects "ctid". "wholerow" is always there. Not
>> for postgres_fdw but for other wrappers it might be a bad news. ctid,
>> whole row obtained from the remote postgres server will fit the tuple
>> descriptor of parent, but for other FDWs the column injected by
>> rewriteTargetListUD() may make the child tuple look different from
>> that of the parent, so we may not pass that column down to the child.
>>
>
> I'm trying to say that when we have a regular table as parent, and
> foreign table as child, in rewriteTargetListUD `wholerow` won't be
> added, because rewriteTargetListUD will be called only for parent
> relation. You can see that by running the script i provided in the first
> message of this thread.

You are right.

explain verbose update parent set val = random();
QUERY PLAN
-------------------------------------------------------------------------------
Update on public.parent (cost=0.00..258.63 rows=5535 width=10)
Update on public.parent
Update on public.child1
Foreign Update on public.ftable
Remote SQL: UPDATE public.child1 SET val = $2 WHERE ctid = $1
-> Seq Scan on public.parent (cost=0.00..4.83 rows=255 width=10)
Output: random(), parent.ctid
-> Seq Scan on public.child1 (cost=0.00..48.25 rows=2550 width=10)
Output: random(), child1.ctid
-> Foreign Scan on public.ftable (cost=100.00..205.55 rows=2730 width=10)
Output: random(), ftable.ctid
Remote SQL: SELECT ctid FROM public.child1 FOR UPDATE
(12 rows)

explain verbose update ftable set val = random();
QUERY PLAN
-------------------------------------------------------------------------------
Update on public.ftable (cost=100.00..159.42 rows=1412 width=38)
Remote SQL: UPDATE public.child1 SET val = $2 WHERE ctid = $1
-> Foreign Scan on public.ftable (cost=100.00..159.42 rows=1412 width=38)
Output: random(), ctid, ftable.*
Remote SQL: SELECT val, ctid FROM public.child1 FOR UPDATE
(5 rows)

Anyway, the problem I am stating, i.e. we have a bigger problem to fix
when there are FDWs other than postgres_fdw involved seems to be still
valid.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company