On 2017/08/07 15:33, Amit Langote wrote:
> On 2017/08/07 15:22, Etsuro Fujita wrote:
>> On 2017/08/07 13:11, Amit Langote wrote:> The patch looks good too.
>> Although, looking at the following hunk:
>>>
>>> + Assert(partrel->rd_rel->relkind == RELKIND_RELATION ||
>>> + partrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE);
>>> +
>>> /*
>>> * Verify result relation is a valid target for the current
>>> operation.
>>> */
>>> ! if (partrel->rd_rel->relkind == RELKIND_RELATION)
>>> ! CheckValidResultRel(partrel, CMD_INSERT);
>>>
>>> makes me now wonder if we need the CheckValidResultRel check at all. The
>>> only check currently in place for RELKIND_RELATION is
>>> CheckCmdReplicaIdentity(), which is a noop (returns true) for inserts
>>> anyway.
>>
>> Good point! I left the verification for a plain table because that is
>> harmless but as you mentioned, that is nothing but an overhead. So, here
>> is a new version which removes the verification at all from
>> ExecSetupPartitionTupleRouting.
>
> The updated patch looks good to me, thanks.
Thanks for the review!
Best regards,
Etsuro Fujita