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

On Tue, 16 May 2017 15:21:27 +0530
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote:

> On Mon, May 15, 2017 at 7:24 PM, Ildus Kurbangaliev
> <i.kurbangaliev@postgrespro.ru> wrote:
> > On Mon, 15 May 2017 17:43:52 +0530
> > Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote:
> >  
> >> On Mon, May 15, 2017 at 2:46 PM, Ildus Kurbangaliev
> >> <i.kurbangaliev@postgrespro.ru> wrote:  
> >> > On Mon, 15 May 2017 10:34:58 +0530
> >> > Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >> >  
> >> >> On Sun, May 14, 2017 at 9:54 PM, Dilip Kumar
> >> >> <dilipbalaut@gmail.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