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

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
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 08:39:53
Message-ID: CAFjFpRfTpamoUz6fNyk6gPh=ecfBJjbUHJNKbDxscpyPJ3FfjQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jun 2, 2017 at 2:40 PM, Etsuro Fujita
<fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> 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 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.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2017-06-05 08:45:51 Update comments in nodeModifyTable.c
Previous Message Heikki Linnakangas 2017-06-05 08:32:00 Re: simplehash.h typo