Re: Oddity in error handling of constraint violation in ExecConstraints for partitioned tables

Fujita-san,

On 2017/07/10 14:15, Etsuro Fujita wrote:
> On 2017/07/07 18:47, Amit Langote wrote:
>> On 2017/07/06 16:06, Etsuro Fujita wrote:
>>> I think this should be fixed. Attached is a patch for that.
>
>> How about setting ri_RangeTableIndex of the partition ResultRelInfo
>> correctly in the first place? If you look at the way InitResultRelInfo()
>> is called in ExecSetupPartitionTupleRouting(), a dummy RT index of 1 is
>> passed. We could instead pass the correct one. I think
>> ModifyTable.nominalRelation is that (at least in the INSERT case.
>>
>> The attached patch implements that. It modifies
>> ExecSetupPartitionTupleRouting to accept one more argument resultRTindex
>> and passes along the same to InitResultRelInfo(). Later when
>> ExecConstraints() wants to build the modifiedCols set, it looks up the
>> correct RTE using the partition ResultRelInfo, which has the appropriate
>> ri_RangeTableIndex set and uses the same.
>
> Looks good to me.

Thanks for the review.

>> The patch keeps tests that you added in your patch. Added this to the
>> open items list.
>
> Another thing I noticed is the error handling in ExecWithCheckOptions; it
> doesn't take any care for partition tables, so the column description in
> the error message for WCO_VIEW_CHECK is built using the partition table's
> rowtype, not the root table's. But I think it'd be better to build that
> using the root table's rowtype, like ExecConstraints, because that would
> make the column description easier to understand since the parent view(s)
> (from which WithCheckOptions evaluated there are created) would have been
> defined on the root table. This seems independent from the above issue,
> so I created a separate patch, which I'm attaching. What do you think
> about that?

Good catch. The patch looks spot on to me. To keep both the patches
together, I'm attaching them here as 0001 which fixes the original issue
you reported on this thread and 0002 which is your patch to fix error
reporting in ExecWithCheckOptions.

Thanks,
Amit