Re: Needless additional partition check in INSERT?

On 2018/05/10 14:42, Simon Riggs wrote:
> On 10 May 2018 at 05:33, David Rowley <david.rowley@2ndquadrant.com> wrote:
>> On 10 May 2018 at 16:13, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> The patch to ExecInsert looks good, but I think we also need to do the
>>> same thing in CopyFrom.
>>
>> I think so too.
>>
>> Updated patch attached.
> 
> Patch is good.
> 
> The cause of this oversight is the lack of comments to explain the
> original coding, so we need to correct that in this patch, please.

There does exist a comment but perhaps not where one would expect, that
is, not immediately above the call to ExecConstraints.  The following
comment is written in both CopyFrom and ExecInsert just above where
check_partition_constr variable is set.

          /*
           * We always check the partition constraint, including when
           * the tuple got here via tuple-routing.  However we don't
           * need to in the latter case if no BR trigger is defined on
           * the partition.  Note that a BR trigger might modify the
           * tuple such that the partition constraint is no longer
           * satisfied, so we need to check in that case.
           */

Thanks,
Amit