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



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