mirror of
https://github.com/postgres/postgres.git
synced 2025-06-27 23:21:58 +03:00
Don't needlessly check the partition contraint twice
Starting with commitf0e44751d7
, ExecConstraints was in charge of running the partition constraint; commit19c47e7c82
modified that so that caller could request to skip that checking depending on some conditions, but that commit and15ce775faa
together introduced a small bug there which caused ExecInsert to request skipping the constraint check but have this not be honored -- in effect doing the check twice. This could have been fixed in a very small patch, but on further analysis of the involved function and its callsites, it turns out to be simpler to give the responsibility of checking the partition constraint fully to the caller, and return ExecConstraints to its original (pre-partitioning) shape where it only checked tuple descriptor-related constraints. Each caller must do partition constraint checking on its own schedule, which is more convenient after commit2f17844104
anyway. Reported-by: David Rowley Author: David Rowley, Álvaro Herrera Reviewed-by: Amit Langote, Amit Khandekar, Simon Riggs Discussion: https://postgr.es/m/CAKJS1f8w8+awsxgea8wt7_UX8qzOQ=Tm1LD+U1fHqBAkXxkW2w@mail.gmail.com
This commit is contained in:
@ -365,16 +365,6 @@ ExecInsert(ModifyTableState *mtstate,
|
||||
else
|
||||
{
|
||||
WCOKind wco_kind;
|
||||
bool check_partition_constr;
|
||||
|
||||
/*
|
||||
* 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.
|
||||
*/
|
||||
check_partition_constr = (resultRelInfo->ri_PartitionCheck != NIL);
|
||||
|
||||
/*
|
||||
* Constraints might reference the tableoid column, so initialize
|
||||
@ -402,17 +392,21 @@ ExecInsert(ModifyTableState *mtstate,
|
||||
ExecWithCheckOptions(wco_kind, resultRelInfo, slot, estate);
|
||||
|
||||
/*
|
||||
* No need though if the tuple has been routed, and a BR trigger
|
||||
* doesn't exist.
|
||||
* Check the constraints of the tuple.
|
||||
*/
|
||||
if (resultRelInfo->ri_PartitionRoot != NULL &&
|
||||
!(resultRelInfo->ri_TrigDesc &&
|
||||
resultRelInfo->ri_TrigDesc->trig_insert_before_row))
|
||||
check_partition_constr = false;
|
||||
if (resultRelationDesc->rd_att->constr)
|
||||
ExecConstraints(resultRelInfo, slot, estate);
|
||||
|
||||
/* Check the constraints of the tuple */
|
||||
if (resultRelationDesc->rd_att->constr || check_partition_constr)
|
||||
ExecConstraints(resultRelInfo, slot, estate, true);
|
||||
/*
|
||||
* Also check the tuple against the partition constraint, if there is
|
||||
* one; except that if we got here via tuple-routing, we don't need to
|
||||
* if there's no BR trigger defined on the partition.
|
||||
*/
|
||||
if (resultRelInfo->ri_PartitionCheck &&
|
||||
(resultRelInfo->ri_PartitionRoot == NULL ||
|
||||
(resultRelInfo->ri_TrigDesc &&
|
||||
resultRelInfo->ri_TrigDesc->trig_insert_before_row)))
|
||||
ExecPartitionCheck(resultRelInfo, slot, estate, true);
|
||||
|
||||
if (onconflict != ONCONFLICT_NONE && resultRelInfo->ri_NumIndices > 0)
|
||||
{
|
||||
@ -1037,7 +1031,7 @@ lreplace:;
|
||||
*/
|
||||
partition_constraint_failed =
|
||||
resultRelInfo->ri_PartitionCheck &&
|
||||
!ExecPartitionCheck(resultRelInfo, slot, estate);
|
||||
!ExecPartitionCheck(resultRelInfo, slot, estate, false);
|
||||
|
||||
if (!partition_constraint_failed &&
|
||||
resultRelInfo->ri_WithCheckOptions != NIL)
|
||||
@ -1168,7 +1162,7 @@ lreplace:;
|
||||
* have it validate all remaining checks.
|
||||
*/
|
||||
if (resultRelationDesc->rd_att->constr)
|
||||
ExecConstraints(resultRelInfo, slot, estate, false);
|
||||
ExecConstraints(resultRelInfo, slot, estate);
|
||||
|
||||
/*
|
||||
* replace the heap tuple
|
||||
|
Reference in New Issue
Block a user