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

2017-09-12 Thread Ildus Kurbangaliev
On Mon, 24 Jul 2017 11:59:13 +0900 Etsuro Fujita wrote: > On 2017/07/21 19:16, Etsuro Fujita wrote: > > On 2017/07/20 11:21, Etsuro Fujita wrote: > >> On 2017/07/19 23:36, Tom Lane wrote: > >>> Please put the responsibility of doing const-expression > >>>

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

2017-07-23 Thread Etsuro Fujita
On 2017/07/21 19:16, Etsuro Fujita wrote: On 2017/07/20 11:21, Etsuro Fujita wrote: On 2017/07/19 23:36, Tom Lane wrote: Please put the responsibility of doing const-expression simplification in these cases somewhere closer to where the problem is being created. It would be reasonable that

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

2017-07-21 Thread Etsuro Fujita
On 2017/07/20 11:21, Etsuro Fujita wrote: On 2017/07/19 23:36, Tom Lane wrote: Please put the responsibility of doing const-expression simplification in these cases somewhere closer to where the problem is being created. It would be reasonable that it's the FDW's responsibility to do that

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

2017-07-19 Thread Etsuro Fujita
On 2017/07/19 23:36, Tom Lane wrote: Etsuro Fujita writes: * Modified rewrite_targetlist(), which is a new function added to preptlist.c, so that we do const-simplification to junk TLEs that AddForeignUpdateTargets() added, as that API allows the FDW to add junk

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

2017-07-19 Thread Tom Lane
Etsuro Fujita writes: > * Modified rewrite_targetlist(), which is a new function added to > preptlist.c, so that we do const-simplification to junk TLEs that > AddForeignUpdateTargets() added, as that API allows the FDW to add junk > TLEs containing non-Var

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

2017-07-19 Thread Etsuro Fujita
On 2017/07/13 21:10, Etsuro Fujita wrote: Attached is an updated version of the patch. Here is an updated version of the patch. Changes are: * Modified rewrite_targetlist(), which is a new function added to preptlist.c, so that we do const-simplification to junk TLEs that

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

2017-07-13 Thread Etsuro Fujita
On 2017/06/30 18:44, Etsuro Fujita wrote: On 2017/06/16 21:29, Etsuro Fujita wrote: I'll have second thought about this, so I'll mark this as waiting on author. I spent quite a bit of time on this and came up with a solution for addressing the concern mentioned by Ashutosh [1]. The basic

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

2017-06-30 Thread Etsuro Fujita
On 2017/06/16 21:29, Etsuro Fujita wrote: On 2017/06/16 19:26, Ashutosh Bapat wrote: That issue has not been addressed. The reason stated was that it would make code complicated. But I have not had chance to look at how complicated would be and assess myself whether that's worth the trouble.

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

2017-06-16 Thread Etsuro Fujita
On 2017/06/16 19:26, Ashutosh Bapat wrote: On Fri, Jun 16, 2017 at 3:35 PM, Etsuro Fujita Ashutosh mentioned his concern about what I proposed above before [2], but I'm not sure we should address that. And there have been no opinions from him (or anyone else) since then. So, I'd like to leave

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

2017-06-16 Thread Etsuro Fujita
On 2017/06/16 19:26, Ashutosh Bapat wrote: Also, I don't see any discussion about my concern [3] about a parent with child from multiple foreign servers with different FDWs. So, I am not sure whether the patch really fixes the problem in its entirety. The patch would allow child tables to have

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

2017-06-16 Thread Ashutosh Bapat
On Fri, Jun 16, 2017 at 3:35 PM, Etsuro Fujita wrote: > On 2017/06/16 0:05, Ildus Kurbangaliev wrote: > > > I wrote: > > One approach I came up with to fix this issue is to rewrite the > targetList entries of an inherited UPDATE/DELETE properly using >

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

2017-06-16 Thread Etsuro Fujita
On 2017/06/16 0:05, Ildus Kurbangaliev wrote: I 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

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

2017-06-15 Thread Ildus Kurbangaliev
On Mon, 5 Jun 2017 17:25:27 +0900 Etsuro Fujita 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

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

2017-06-05 Thread Etsuro Fujita
On 2017/06/05 17:39, Ashutosh Bapat wrote: On Fri, Jun 2, 2017 at 2:40 PM, Etsuro Fujita 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

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

2017-06-05 Thread Ashutosh Bapat
On Fri, Jun 2, 2017 at 2:40 PM, 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

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

2017-06-05 Thread Etsuro Fujita
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

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

2017-06-02 Thread Etsuro Fujita
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.

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

2017-05-17 Thread Etsuro Fujita
On 2017/05/17 17:54, Ildus Kurbangaliev wrote: On Wed, 17 May 2017 15:28:24 +0900 Michael Paquier wrote: On Tue, May 16, 2017 at 11:26 PM, Ildus Kurbangaliev wrote: On Tue, 16 May 2017 21:36:11 +0900 Etsuro Fujita

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

2017-05-17 Thread Ildus Kurbangaliev
On Wed, 17 May 2017 15:28:24 +0900 Michael Paquier wrote: > On Tue, May 16, 2017 at 11:26 PM, Ildus Kurbangaliev > wrote: > > On Tue, 16 May 2017 21:36:11 +0900 > > Etsuro Fujita wrote: > >> On 2017/05/16

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

2017-05-17 Thread Michael Paquier
On Tue, May 16, 2017 at 11:26 PM, Ildus Kurbangaliev wrote: > On Tue, 16 May 2017 21:36:11 +0900 > Etsuro Fujita wrote: >> On 2017/05/16 21:11, Ashutosh Bapat wrote: >> > On Tue, May 16, 2017 at 5:35 PM, Ildus Kurbangaliev >> >

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

2017-05-16 Thread Ildus Kurbangaliev
On Tue, 16 May 2017 21:36:11 +0900 Etsuro Fujita wrote: > On 2017/05/16 21:11, Ashutosh Bapat wrote: > > On Tue, May 16, 2017 at 5:35 PM, Ildus Kurbangaliev > > wrote: > > >> I agree. Maybe this issue should be added to Postgresql

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

2017-05-16 Thread Etsuro Fujita
On 2017/05/16 21:11, Ashutosh Bapat wrote: On Tue, May 16, 2017 at 5:35 PM, Ildus Kurbangaliev 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

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

2017-05-16 Thread Ashutosh Bapat
On Tue, May 16, 2017 at 5:35 PM, Ildus Kurbangaliev 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

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

2017-05-16 Thread Ildus Kurbangaliev
On Tue, 16 May 2017 15:21:27 +0530 Ashutosh Bapat wrote: > On Mon, May 15, 2017 at 7:24 PM, Ildus Kurbangaliev > wrote: > > On Mon, 15 May 2017 17:43:52 +0530 > > Ashutosh Bapat wrote: > > > >>

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

2017-05-16 Thread Ashutosh Bapat
On Mon, May 15, 2017 at 7:24 PM, Ildus Kurbangaliev wrote: > On Mon, 15 May 2017 17:43:52 +0530 > Ashutosh Bapat wrote: > >> On Mon, May 15, 2017 at 2:46 PM, Ildus Kurbangaliev >> wrote: >> > On Mon,

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

2017-05-15 Thread Ildus Kurbangaliev
On Mon, 15 May 2017 17:43:52 +0530 Ashutosh Bapat wrote: > On Mon, May 15, 2017 at 2:46 PM, Ildus Kurbangaliev > wrote: > > On Mon, 15 May 2017 10:34:58 +0530 > > Dilip Kumar wrote: > > > >> On Sun, May

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

2017-05-15 Thread Ashutosh Bapat
On Mon, May 15, 2017 at 6:04 PM, Dilip Kumar wrote: > On Mon, May 15, 2017 at 5:43 PM, Ashutosh Bapat > wrote: >> Yes. postgresAddForeignUpdateTargets() which is called by >> rewriteTargetListUD injects "ctid". "wholerow" is always there.

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

2017-05-15 Thread Dilip Kumar
On Mon, May 15, 2017 at 5:43 PM, Ashutosh Bapat wrote: > 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

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

2017-05-15 Thread Ashutosh Bapat
On Mon, May 15, 2017 at 2:46 PM, Ildus Kurbangaliev wrote: > On Mon, 15 May 2017 10:34:58 +0530 > Dilip Kumar wrote: > >> On Sun, May 14, 2017 at 9:54 PM, Dilip Kumar >> wrote: >> > After your fix, now tupleid is

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

2017-05-15 Thread Dilip Kumar
On Mon, May 15, 2017 at 2:46 PM, Ildus Kurbangaliev wrote: > 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

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

2017-05-15 Thread Ildus Kurbangaliev
On Mon, 15 May 2017 10:34:58 +0530 Dilip Kumar wrote: > On Sun, May 14, 2017 at 9:54 PM, Dilip Kumar > 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

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

2017-05-14 Thread Michael Paquier
On Mon, May 15, 2017 at 2:04 PM, Dilip Kumar wrote: > On Sun, May 14, 2017 at 9:54 PM, Dilip Kumar 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

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

2017-05-14 Thread Dilip Kumar
On Sun, May 14, 2017 at 9:54 PM, Dilip Kumar 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

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

2017-05-14 Thread Dilip Kumar
On Sun, May 14, 2017 at 5:35 PM, Ildus Kurbangaliev wrote: > TRAP: FailedAssertion("!(((const void*)(fdw_trigtuple) != ((void *)0)) > ^ ((bool) (((const void*)(tupleid) != ((void *)0)) && > ((tupleid)->ip_posid != 0", File: "trigger.c", Line: 2428) > > I'm not