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