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

On Wed, 17 May 2017 15:28:24 +0900
Michael Paquier <michael.paquier@gmail.com> wrote:

> On Tue, May 16, 2017 at 11:26 PM, Ildus Kurbangaliev
> <i.kurbangaliev@postgrespro.ru> wrote:
> > On Tue, 16 May 2017 21:36:11 +0900
> > Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:  
> >> On 2017/05/16 21:11, Ashutosh Bapat wrote:  
> >> > On Tue, May 16, 2017 at 5:35 PM, Ildus Kurbangaliev
> >> > <i.kurbangaliev@postgrespro.ru> wrote:  
> >>  
> >> >> 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.  
> >>  
> >> > I doubt if this is an open item, since DMLs on foreign tables are
> >> > supported since 9.3 and support to add foreign tables to
> >> > inheritance was added back in 9.5.  
> >>
> >> I think this issue was introduced by the latter, so that was my
> >> fault.
> >>
> >> One approach I came up with to fix this issue is to rewrite the
> >> targetList entries of an inherited UPDATE/DELETE properly using
> >> rewriteTargetListUD, when generating a plan for each child table in
> >> inheritance_planner.  Attached is a WIP patch for that.  Maybe I am
> >> missing something, though.  
> 
> Could this patch include some regression tests to see at what extent
> it has been tested? We surely don't want to see that broken again in
> the future as well. (Nit: I did not look at the patch in details yet)
> 
> > I tested the patch, looks good.  
> 
> What kind of tests did you do?

I tested update triggers for foreign table when regular table is a
parent and foreign table is a child. Case like this:

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 // we have triggers on ftable here

> 
>                   junkfilter = resultRelInfo->ri_junkFilter;
> +                 tupleid = NULL;
>                   estate->es_result_relation_info = resultRelInfo;
> Er, what is that?

That fixes the bug when tupleid from regular tuple is used to get
oldtuple for triggers of foreign table.

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