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

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.
>
> Looking up the ResultRelInfo using the proposed method in
> ExecFindResultRelInfo() can be unreliable sometimes, that is, the method
> of using the root table OID to pin down the RangeTableEntry to get the the
> modified columns set.

You are right. Thank you for pointing that out.

> 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.

> 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?

Best regards,
Etsuro Fujita