Re: Needless additional partition check in INSERT?

On 2018/05/10 12:55, David Rowley wrote:
> Hi,
>
> Scanning ExecInsert, it looks like there's a needless additional
> partition constraint check against the tuple. This should only be
> required if there's a before row INSERT trigger. The code block up
> one from the additional check tries to disable the check, but it goes
> ahead regardless, providing there's some other constraint.
>
> ExecFindPartition should have already located the correct partition
> and nothing should have changed in the absence of before row insert
> triggers, so it looks like we're fine to not bother re-checking.

I think you're right. So if we call ExecConstraints only because there
are other tuple constraints (rd_att->constr != NULL), then currently we
pass true for whether to check the partition constraint, irrespective of
the value of check_partition_constr. I guess 19c47e7c820 should have done
it the way your patch teaches it do to begin with.

The patch to ExecInsert looks good, but I think we also need to do the
same thing in CopyFrom.

Thanks,
Amit