From 829cbc0c03bc3b51b12aaff3feb0161366a9d03b Mon Sep 17 00:00:00 2001 From: vagrant Date: Mon, 24 Dec 2018 23:40:17 +0000 Subject: [PATCH] Refactor per-row unique key deferred constraint triggers into per-statement triggers. The function unique_key_recheck() will instead walk the transition table and individually check rows. The hope is that this will result in reduced locking and a speed improvement for operations with many rows affected. --- src/backend/catalog/index.c | 68 ++++++++---- src/backend/commands/constraint.c | 169 ++++++++++++++++-------------- src/backend/commands/trigger.c | 9 -- 3 files changed, 136 insertions(+), 110 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 8709e8c22c..7a8da5b1d9 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1363,26 +1363,54 @@ index_constraint_create(Relation heapRelation, */ if (deferrable) { - CreateTrigStmt *trigger; - - trigger = makeNode(CreateTrigStmt); - trigger->trigname = (constraintType == CONSTRAINT_PRIMARY) ? - "PK_ConstraintTrigger" : - "Unique_ConstraintTrigger"; - trigger->relation = NULL; - trigger->funcname = SystemFuncName("unique_key_recheck"); - trigger->args = NIL; - trigger->row = true; - trigger->timing = TRIGGER_TYPE_AFTER; - trigger->events = TRIGGER_TYPE_INSERT | TRIGGER_TYPE_UPDATE; - trigger->columns = NIL; - trigger->whenClause = NULL; - trigger->isconstraint = true; - trigger->deferrable = true; - trigger->initdeferred = initdeferred; - trigger->constrrel = NULL; - - (void) CreateTrigger(trigger, NULL, RelationGetRelid(heapRelation), + CreateTrigStmt *insert_trigger, *update_trigger; + + TriggerTransition *ins = makeNode(TriggerTransition); + ins->name = "pg_inserted_transition_table"; + ins->isNew = true; + ins->isTable = true; + + insert_trigger = makeNode(CreateTrigStmt); + insert_trigger->trigname = (constraintType == CONSTRAINT_PRIMARY) ? + "PK_ConstraintTrigger_i" : + "Unique_ConstraintTrigger_i"; + insert_trigger->relation = NULL; + insert_trigger->funcname = SystemFuncName("unique_key_recheck"); + insert_trigger->args = NIL; + insert_trigger->row = false; + insert_trigger->timing = TRIGGER_TYPE_AFTER; + insert_trigger->events = TRIGGER_TYPE_INSERT; + insert_trigger->columns = NIL; + insert_trigger->whenClause = NULL; + insert_trigger->isconstraint = true; + insert_trigger->deferrable = true; + insert_trigger->initdeferred = initdeferred; + insert_trigger->constrrel = NULL; + insert_trigger->transitionRels = list_make1(ins); + + (void) CreateTrigger(insert_trigger, NULL, RelationGetRelid(heapRelation), + InvalidOid, conOid, indexRelationId, InvalidOid, + InvalidOid, NULL, true, false); + + update_trigger = makeNode(CreateTrigStmt); + update_trigger->trigname = (constraintType == CONSTRAINT_PRIMARY) ? + "PK_ConstraintTrigger_u" : + "Unique_ConstraintTrigger_u"; + update_trigger->relation = NULL; + update_trigger->funcname = SystemFuncName("unique_key_recheck"); + update_trigger->args = NIL; + update_trigger->row = false; + update_trigger->timing = TRIGGER_TYPE_AFTER; + update_trigger->events = TRIGGER_TYPE_UPDATE; + update_trigger->columns = NIL; + update_trigger->whenClause = NULL; + update_trigger->isconstraint = true; + update_trigger->deferrable = true; + update_trigger->initdeferred = initdeferred; + update_trigger->constrrel = NULL; + update_trigger->transitionRels = list_make1(ins); + + (void) CreateTrigger(update_trigger, NULL, RelationGetRelid(heapRelation), InvalidOid, conOid, indexRelationId, InvalidOid, InvalidOid, NULL, true, false); } diff --git a/src/backend/commands/constraint.c b/src/backend/commands/constraint.c index b0b2cb2a14..c5d2d1f983 100644 --- a/src/backend/commands/constraint.c +++ b/src/backend/commands/constraint.c @@ -45,7 +45,7 @@ unique_key_recheck(PG_FUNCTION_ARGS) IndexInfo *indexInfo; EState *estate; ExprContext *econtext; - TupleTableSlot *slot; + TupleTableSlot *slot = NULL; Datum values[INDEX_MAX_KEYS]; bool isnull[INDEX_MAX_KEYS]; @@ -61,54 +61,18 @@ unique_key_recheck(PG_FUNCTION_ARGS) funcname))); if (!TRIGGER_FIRED_AFTER(trigdata->tg_event) || - !TRIGGER_FIRED_FOR_ROW(trigdata->tg_event)) + !TRIGGER_FIRED_FOR_STATEMENT(trigdata->tg_event)) ereport(ERROR, (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED), - errmsg("function \"%s\" must be fired AFTER ROW", + errmsg("function \"%s\" must be fired AFTER STATEMENT", funcname))); - /* - * Get the new data that was inserted/updated. - */ - if (TRIGGER_FIRED_BY_INSERT(trigdata->tg_event)) - new_row = trigdata->tg_trigtuple; - else if (TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event)) - new_row = trigdata->tg_newtuple; - else - { + if (! TRIGGER_FIRED_BY_INSERT(trigdata->tg_event) && + ! TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event)) ereport(ERROR, (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED), errmsg("function \"%s\" must be fired for INSERT or UPDATE", funcname))); - new_row = NULL; /* keep compiler quiet */ - } - - /* - * If the new_row is now dead (ie, inserted and then deleted within our - * transaction), we can skip the check. However, we have to be careful, - * because this trigger gets queued only in response to index insertions; - * which means it does not get queued for HOT updates. The row we are - * called for might now be dead, but have a live HOT child, in which case - * we still need to make the check --- effectively, we're applying the - * check against the live child row, although we can use the values from - * this row since by definition all columns of interest to us are the - * same. - * - * This might look like just an optimization, because the index AM will - * make this identical test before throwing an error. But it's actually - * needed for correctness, because the index AM will also throw an error - * if it doesn't find the index entry for the row. If the row's dead then - * it's possible the index entry has also been marked dead, and even - * removed. - */ - tmptid = new_row->t_self; - if (!heap_hot_search(&tmptid, trigdata->tg_relation, SnapshotSelf, NULL)) - { - /* - * All rows in the HOT chain are dead, so skip the check. - */ - return PointerGetDatum(NULL); - } /* * Open the index, acquiring a RowExclusiveLock, just as if we were going @@ -119,14 +83,6 @@ unique_key_recheck(PG_FUNCTION_ARGS) RowExclusiveLock); indexInfo = BuildIndexInfo(indexRel); - /* - * The heap tuple must be put into a slot for FormIndexDatum. - */ - slot = MakeSingleTupleTableSlot(RelationGetDescr(trigdata->tg_relation), - &TTSOpsHeapTuple); - - ExecStoreHeapTuple(new_row, slot, false); - /* * Typically the index won't have expressions, but if it does we need an * EState to evaluate them. We need it for exclusion constraints too, @@ -137,49 +93,101 @@ unique_key_recheck(PG_FUNCTION_ARGS) { estate = CreateExecutorState(); econtext = GetPerTupleExprContext(estate); - econtext->ecxt_scantuple = slot; } else estate = NULL; /* - * Form the index values and isnull flags for the index entry that we need - * to check. - * - * Note: if the index uses functions that are not as immutable as they are - * supposed to be, this could produce an index tuple different from the - * original. The index AM can catch such errors by verifying that it - * finds a matching index entry with the tuple's TID. For exclusion - * constraints we check this in check_exclusion_constraint(). + * Iterate over transition table. */ - FormIndexDatum(indexInfo, slot, estate, values, isnull); + tuplestore_select_read_pointer(trigdata->tg_newtable,0); + if (!tuplestore_gettupleslot(trigdata->tg_newtable, true, true, slot)) + ereport(ERROR, + (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED), + errmsg("unexpected end of tuplestore"))); - /* - * Now do the appropriate check. - */ - if (indexInfo->ii_ExclusionOps == NULL) + ereport(ERROR, (errmsg_internal("start walking transition table"))); + + while (! TupIsNull(slot)) { + ereport(WARNING, (errmsg_internal("start loop transition table"))); + new_row = slot->tts_ops->get_heap_tuple(slot); + /* - * Note: this is not a real insert; it is a check that the index entry - * that has already been inserted is unique. Passing t_self is - * correct even if t_self is now dead, because that is the TID the - * index will know about. + * If the new_row is now dead (ie, inserted and then deleted within our + * transaction), we can skip the check. However, we have to be careful, + * because this trigger gets queued only in response to index insertions; + * which means it does not get queued for HOT updates. The row we are + * called for might now be dead, but have a live HOT child, in which case + * we still need to make the check --- effectively, we're applying the + * check against the live child row, although we can use the values from + * this row since by definition all columns of interest to us are the + * same. + * + * This might look like just an optimization, because the index AM will + * make this identical test before throwing an error. But it's actually + * needed for correctness, because the index AM will also throw an error + * if it doesn't find the index entry for the row. If the row's dead then + * it's possible the index entry has also been marked dead, and even + * removed. */ - index_insert(indexRel, values, isnull, &(new_row->t_self), - trigdata->tg_relation, UNIQUE_CHECK_EXISTING, - indexInfo); - } - else - { + tmptid = new_row->t_self; + if (!heap_hot_search(&tmptid, trigdata->tg_relation, SnapshotSelf, NULL)) + { + /* + * All rows in the HOT chain are dead, so skip the check. + */ + continue; + } + + if (estate != NULL) + econtext->ecxt_scantuple = slot; + + /* + * Form the index values and isnull flags for the index entry that we need + * to check. + * + * Note: if the index uses functions that are not as immutable as they are + * supposed to be, this could produce an index tuple different from the + * original. The index AM can catch such errors by verifying that it + * finds a matching index entry with the tuple's TID. For exclusion + * constraints we check this in check_exclusion_constraint(). + */ + FormIndexDatum(indexInfo, slot, estate, values, isnull); + /* - * For exclusion constraints we just do the normal check, but now it's - * okay to throw error. In the HOT-update case, we must use the live - * HOT child's TID here, else check_exclusion_constraint will think - * the child is a conflict. + * Now do the appropriate check. */ - check_exclusion_constraint(trigdata->tg_relation, indexRel, indexInfo, - &tmptid, values, isnull, - estate, false); + if (indexInfo->ii_ExclusionOps == NULL) + { + /* + * Note: this is not a real insert; it is a check that the index entry + * that has already been inserted is unique. Passing t_self is + * correct even if t_self is now dead, because that is the TID the + * index will know about. + */ + index_insert(indexRel, values, isnull, &(new_row->t_self), + trigdata->tg_relation, UNIQUE_CHECK_EXISTING, + indexInfo); + } + else + { + /* + * For exclusion constraints we just do the normal check, but now it's + * okay to throw error. In the HOT-update case, we must use the live + * HOT child's TID here, else check_exclusion_constraint will think + * the child is a conflict. + */ + check_exclusion_constraint(trigdata->tg_relation, indexRel, indexInfo, + &tmptid, values, isnull, + estate, false); + } + + ExecDropSingleTupleTableSlot(slot); + + ereport(WARNING, (errmsg_internal("end loop transition table"))); + if (!tuplestore_gettupleslot(trigdata->tg_newtable, true, true, slot)) + break; } /* @@ -189,7 +197,6 @@ unique_key_recheck(PG_FUNCTION_ARGS) if (estate != NULL) FreeExecutorState(estate); - ExecDropSingleTupleTableSlot(slot); index_close(indexRel, RowExclusiveLock); diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index fb0de60a45..e0feec747b 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -504,15 +504,6 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("transition tables cannot be specified for triggers with column lists"))); - /* - * We disallow constraint triggers with transition tables, to - * protect the assumption that such triggers can't be deferred. - * See notes with AfterTriggers data structures, below. - * - * Currently this is enforced by the grammar, so just Assert here. - */ - Assert(!stmt->isconstraint); - if (tt->isNew) { if (!(TRIGGER_FOR_INSERT(tgtype) || -- 2.17.1