From 5d57489c6d8c5e1b10413362005ae333acd6c243 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 6 Jun 2018 15:08:23 -0400 Subject: [PATCH] Fix situation with partition constraints --- src/backend/commands/copy.c | 30 +++++++++------------------ src/backend/executor/execMain.c | 32 ++++++++++++++++------------- src/backend/executor/execPartition.c | 5 ++--- src/backend/executor/execReplication.c | 8 ++++++-- src/backend/executor/nodeModifyTable.c | 37 +++++++++++----------------------- src/include/executor/executor.h | 5 ++--- 6 files changed, 49 insertions(+), 68 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 0a1706c0d1..5f4ffd2975 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2726,29 +2726,17 @@ CopyFrom(CopyState cstate) else { /* - * 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. - */ - bool check_partition_constr = - (resultRelInfo->ri_PartitionCheck != NIL); - - if (saved_resultRelInfo != NULL && - !(resultRelInfo->ri_TrigDesc && - resultRelInfo->ri_TrigDesc->trig_insert_before_row)) - check_partition_constr = false; - - /* - * If the target is a plain table, check the constraints of - * the tuple. + * Check the tuple is valid against all constraints, and the + * partition constraint, as required. */ if (resultRelInfo->ri_FdwRoutine == NULL && - (resultRelInfo->ri_RelationDesc->rd_att->constr || - check_partition_constr)) - ExecConstraints(resultRelInfo, slot, estate, true); + resultRelInfo->ri_RelationDesc->rd_att->constr) + ExecConstraints(resultRelInfo, slot, estate); + if (resultRelInfo->ri_PartitionCheck && + (saved_resultRelInfo == NULL || + (resultRelInfo->ri_TrigDesc && + resultRelInfo->ri_TrigDesc->trig_insert_before_row))) + ExecPartitionCheck(resultRelInfo, slot, estate, true); if (useHeapMultiInsert) { diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 3d12f9c76f..c394b5dbed 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1856,14 +1856,17 @@ ExecRelCheck(ResultRelInfo *resultRelInfo, /* * ExecPartitionCheck --- check that tuple meets the partition constraint. * - * Exported in executor.h for outside use. - * Returns true if it meets the partition constraint, else returns false. + * Returns true if it meets the partition constraint. If the constraint + * fails and we're asked to emit to error, do so and don't return; otherwise + * return false. + * */ bool ExecPartitionCheck(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, - EState *estate) + EState *estate, bool emitError) { ExprContext *econtext; + bool success; /* * If first time through, build expression state tree for the partition @@ -1890,7 +1893,13 @@ ExecPartitionCheck(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, * As in case of the catalogued constraints, we treat a NULL result as * success here, not a failure. */ - return ExecCheck(resultRelInfo->ri_PartitionCheckExpr, econtext); + success = ExecCheck(resultRelInfo->ri_PartitionCheckExpr, econtext); + + /* if asked to emit error, don't actually return */ + if (!success && emitError) + ExecPartitionCheckEmitError(resultRelInfo, slot, estate); + + return success; } /* @@ -1951,17 +1960,17 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, /* * ExecConstraints - check constraints of the tuple in 'slot' * - * This checks the traditional NOT NULL and check constraints, and if - * requested, checks the partition constraint. + * This checks the traditional NOT NULL and check constraints. + * + * The partition constraint is *NOT* checked. * * Note: 'slot' contains the tuple to check the constraints of, which may * have been converted from the original input tuple after tuple routing. - * 'resultRelInfo' is the original result relation, before tuple routing. + * 'resultRelInfo' is the final result relation, after tuple routing. */ void ExecConstraints(ResultRelInfo *resultRelInfo, - TupleTableSlot *slot, EState *estate, - bool check_partition_constraint) + TupleTableSlot *slot, EState *estate) { Relation rel = resultRelInfo->ri_RelationDesc; TupleDesc tupdesc = RelationGetDescr(rel); @@ -2076,13 +2085,8 @@ ExecConstraints(ResultRelInfo *resultRelInfo, errtableconstraint(orig_rel, failed))); } } - - if (check_partition_constraint && resultRelInfo->ri_PartitionCheck && - !ExecPartitionCheck(resultRelInfo, slot, estate)) - ExecPartitionCheckEmitError(resultRelInfo, slot, estate); } - /* * ExecWithCheckOptions -- check that tuple satisfies any WITH CHECK OPTIONs * of the specified kind. diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index c83991c93c..bb9bf5afa4 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -201,9 +201,8 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, * First check the root table's partition constraint, if any. No point in * routing the tuple if it doesn't belong in the root table itself. */ - if (resultRelInfo->ri_PartitionCheck && - !ExecPartitionCheck(resultRelInfo, slot, estate)) - ExecPartitionCheckEmitError(resultRelInfo, slot, estate); + if (resultRelInfo->ri_PartitionCheck) + ExecPartitionCheck(resultRelInfo, slot, estate, true); /* start with the root partitioned table */ parent = pd[0]; diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 4fbdfc0a09..41e857e378 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -413,7 +413,9 @@ ExecSimpleRelationInsert(EState *estate, TupleTableSlot *slot) /* Check the constraints of the tuple */ if (rel->rd_att->constr) - ExecConstraints(resultRelInfo, slot, estate, true); + ExecConstraints(resultRelInfo, slot, estate); + if (resultRelInfo->ri_PartitionCheck) + ExecPartitionCheck(resultRelInfo, slot, estate, true); /* Store the slot into tuple that we can inspect. */ tuple = ExecMaterializeSlot(slot); @@ -478,7 +480,9 @@ ExecSimpleRelationUpdate(EState *estate, EPQState *epqstate, /* Check the constraints of the tuple */ if (rel->rd_att->constr) - ExecConstraints(resultRelInfo, slot, estate, true); + ExecConstraints(resultRelInfo, slot, estate); + if (resultRelInfo->ri_PartitionCheck) + ExecPartitionCheck(resultRelInfo, slot, estate, true); /* Store the slot into tuple that we can write. */ tuple = ExecMaterializeSlot(slot); diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index c4c841cdd7..c75d66ab83 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -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,16 @@ 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 tuple is valid against all constraints, and the partition + * constraint, as required. */ - if (resultRelInfo->ri_PartitionRoot != NULL && - !(resultRelInfo->ri_TrigDesc && - resultRelInfo->ri_TrigDesc->trig_insert_before_row)) - check_partition_constr = false; - - /* Check the constraints of the tuple */ - if (resultRelationDesc->rd_att->constr || check_partition_constr) - ExecConstraints(resultRelInfo, slot, estate, true); + if (resultRelationDesc->rd_att->constr) + ExecConstraints(resultRelInfo, slot, estate); + 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 +1026,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) @@ -1166,12 +1155,10 @@ lreplace:; * slot for the orig_slot argument, because unlike ExecInsert(), no * tuple-routing is performed here, hence the slot remains unchanged. * We've already checked the partition constraint above; however, we - * must still ensure the tuple passes all other constraints, so we - * will call ExecConstraints() and have it validate all remaining - * checks. + * must still ensure the tuple passes all other constraints. */ if (resultRelationDesc->rd_att->constr) - ExecConstraints(resultRelInfo, slot, estate, false); + ExecConstraints(resultRelInfo, slot, estate); /* * replace the heap tuple diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index a7ea3c7d10..f82b51667f 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -180,10 +180,9 @@ extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid); extern void ExecCleanUpTriggerState(EState *estate); extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids); extern void ExecConstraints(ResultRelInfo *resultRelInfo, - TupleTableSlot *slot, EState *estate, - bool check_partition_constraint); + TupleTableSlot *slot, EState *estate); extern bool ExecPartitionCheck(ResultRelInfo *resultRelInfo, - TupleTableSlot *slot, EState *estate); + TupleTableSlot *slot, EState *estate, bool emitError); extern void ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); extern void ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, -- 2.11.0