On 2017/08/07 15:45, Etsuro Fujita wrote:
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!

If there are no objections, I'll add this to the open item list for v10.

Best regards,
Etsuro Fujita



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to