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

Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru>

From: Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru>
To: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
Cc: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>, Dilip Kumar <dilipbalaut@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Robert Haas <robertmhaas@gmail.com>
Date: 2017-06-15T15:05:48Z
Lists: pgsql-hackers
On Mon, 5 Jun 2017 17:25:27 +0900
Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:

> On 2017/06/02 18:10, Etsuro Fujita wrote:
> > On 2017/05/16 21:36, Etsuro Fujita wrote:  
> >> 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.  
> > 
> > While updating the patch, I noticed the patch rewrites the UPDATE 
> > targetList incorrectly in some cases; rewrite_inherited_tlist I
> > added to adjust_appendrel_attrs (1) removes all junk items from the
> > targetList and (2) adds junk items for the child table using
> > rewriteTargetListUD, but it's wrong to drop all junk items in cases
> > where there are junk items for some other reasons than
> > rewriteTargetListUD.  Consider junk items containing MULTIEXPR
> > SubLink.  One way I came up with to fix this is to change (1) to
> > only remove junk items with resname; since junk items added by
> > rewriteTargetListUD should have resname (note: we would need
> > resname to call ExecFindJunkAttributeInTlist at execution time!)
> > while other junk items wouldn't have resname (see
> > transformUpdateTargetList), we could correctly replace junk items
> > added by rewriteTargetListUD for the parent with ones for the
> > child, by that change.  I might be missing something, though.
> > Comments welcome.  
> 
> I updated the patch that way.  Please find attached an updated
> version.
> 
> Other changes:
> * Moved the initialization for "tupleid" I added in ExecModifyTable
> as discussed before, which I think is essentially the same as
> proposed by Ildus in [1], since I think that would be more consistent
> with "oldtuple".
> * Added regression tests.
> 
> Anyway I'll add this to the next commitfest.
> 
> Best regards,
> Etsuro Fujita
> 
> [1] 
> https://www.postgresql.org/message-id/20170514150525.0346ba72%40postgrespro.ru

Checked the latest patch. Looks good.
Shouldn't this patch be backported to 9.6 and 10beta? The bug
affects them too.

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


Commits

  1. Fix creation of resjunk tlist entries for inherited mixed UPDATE/DELETE.