Re: [HACKERS] Bug in ExecModifyTable function and trigger issuesfor foreign tables - Mailing list pgsql-hackers

From Ildus Kurbangaliev
Subject Re: [HACKERS] Bug in ExecModifyTable function and trigger issuesfor foreign tables
Date
Msg-id 20170615180548.5e142ddc@wp.localdomain
Whole thread Raw
In response to Re: [HACKERS] Bug in ExecModifyTable function and trigger issues forforeign tables  (Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>)
Responses Re: [HACKERS] Bug in ExecModifyTable function and trigger issues forforeign tables
List 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



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: [HACKERS] intermittent failures in Cygwin from select_parallel tests
Next
From: Peter Eisentraut
Date:
Subject: Re: [HACKERS] Getting server crash on Windows when using ICUcollation