Re: Needless additional partition check in INSERT?

Amit Langote <langote_amit_f8@lab.ntt.co.jp>

From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: David Rowley <david.rowley@2ndquadrant.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Robert Haas <robertmhaas@gmail.com>, Amit Langote <amitlangote09@gmail.com>, Amit Khandekar <amitdkhan.pg@gmail.com>
Date: 2018-05-10T04:13:07Z
Lists: pgsql-hackers
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



Commits

  1. Don't needlessly check the partition contraint twice