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

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: Bug in ExecModifyTable function and trigger issues for foreign tables
Date: 2017-06-05 09:25:26
Message-ID: dd5f334d-d4e5-074c-8f1e-1c0fd9c9310a@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 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 haven't looked at the patch, but that doesn't look right. In future
> some code path other than rewriteTargetListUD() may add junk items
> with resname and this fix will remove those junk items as well.

Yeah, I thought that too. I think we could replace junk tlist entries
added by rewriteTargetListUD() more safely, by adding a lot more code,
but I'm not sure it's worth complicating the code at the current stage.

Best regards,
Etsuro Fujita

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2017-06-05 11:16:33 Re: retry shm attach for windows (WAS: Re: OK, so culicidae is *still* broken)
Previous Message Sandro Santilli 2017-06-05 09:23:57 Re: make check false success