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