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

On Tue, 16 May 2017 15:21:27 +0530
Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:

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

I agree. Maybe this issue should be added to Postgresql Open Items?
I think there should be some complex solution that fixes not only
triggers for foreign tables at table partitioning, but covers other
possible not working cases.

--
---
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company