Re: Update comment in ExecPartitionCheck

On 2017/08/26 2:28, Robert Haas wrote:
> On Tue, Jul 4, 2017 at 4:55 AM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> This comment in an error handling in ExecPartitionCheck():
>>
>> if (!ExecCheck(resultRelInfo->ri_PartitionCheckExpr, econtext))
>> {
>> char *val_desc;
>> Relation orig_rel = rel;
>>
>> /* See the comment above. */
>> if (resultRelInfo->ri_PartitionRoot)
>>
>> should be updated because we don't have any comment on that above in the
>> code. Since we have a comment on that in ExecConstraints() defined just
>> below that function, I think the comment should be something like this: "See
>> the comment in ExecConstraints().". Attached is a patch for that.
>
> Hrm. I'm not sure I understand which comment in ExecConstraints()
> this is supposed to refer to. Maybe we need to think a bit harder
> about how to make this clear.

The comment in ExecConstraints is this:

/*
* If the tuple has been routed, it's been converted to the
* partition's rowtype, which might differ from the root
* table's. We must convert it back to the root table's
* rowtype so that val_desc shown error message matches the
* input tuple.
*/
if (resultRelInfo->ri_PartitionRoot)

How about replacing the comment "See the comment above." in
ExecPartitionCheck with something like this: "If the tuple has been
routed, convert it from the partition's rowtype to the root table's. See
the comment in ExecConstraints().". I think that would make it easy to
specify that comment in ExecConstrains. I'd like to propose to update
the same comments in other places as well, just for consistency.

PFA an updated version of the patch.

Best regards,
Etsuro Fujita