From f0799ace07b785160d77634cb9e9ede9ab8cc0a5 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Tue, 18 Jan 2022 10:51:26 -0300 Subject: [PATCH v13 2/2] alvherre tweaks --- src/backend/commands/trigger.c | 161 ++++++++++++++----------- src/backend/executor/execMain.c | 2 +- src/backend/executor/nodeModifyTable.c | 47 ++++---- src/include/executor/executor.h | 2 +- 4 files changed, 118 insertions(+), 94 deletions(-) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 0544efab56..e6aa36a9d8 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -3100,15 +3100,16 @@ ExecARUpdateTriggers(EState *estate, ModifyTableState *mtstate, * separately for DELETE and INSERT to capture transition table rows. * In such case, either old tuple or new tuple can be NULL. */ - TupleTableSlot *oldslot = ExecGetTriggerOldSlot(estate, - src_partinfo != NULL ? - src_partinfo : - relinfo); + TupleTableSlot *oldslot; + ResultRelInfo *tupsrc; + + tupsrc = src_partinfo ? src_partinfo : relinfo; + oldslot = ExecGetTriggerOldSlot(estate, tupsrc); if (fdw_trigtuple == NULL && ItemPointerIsValid(tupleid)) GetTupleForTrigger(estate, NULL, - src_partinfo != NULL ? src_partinfo : relinfo, + tupsrc, tupleid, LockTupleExclusive, oldslot, @@ -3530,9 +3531,9 @@ typedef SetConstraintStateData *SetConstraintState; * Per-trigger-event data * * The actual per-event data, AfterTriggerEventData, includes DONE/IN_PROGRESS - * status bits and up to two tuple CTIDs. Each event record also has an - * associated AfterTriggerSharedData that is shared across all instances of - * similar events within a "chunk". + * status bits, up to two tuple CTIDs, and optionally two OIDs of partitions + * Each event record also has an associated AfterTriggerSharedData that is + * shared across all instances of similar events within a "chunk". * * For row-level triggers, we arrange not to waste storage on unneeded ctid * fields. Updates of regular tables use two; inserts and deletes of regular @@ -3543,6 +3544,10 @@ typedef SetConstraintStateData *SetConstraintState; * tuple(s). This permits storing tuples once regardless of the number of * row-level triggers on a foreign table. * + * When updates move tuples in partitioned tables to different partitions, + * the OIDs of both partitions are stored too, so that the tuples can be + * fetched. + * * Note that we need triggers on foreign tables to be fired in exactly the * order they were queued, so that the tuples come out of the tuplestore in * the right order. To ensure that, we forbid deferrable (constraint) @@ -3566,16 +3571,15 @@ typedef SetConstraintStateData *SetConstraintState; typedef uint32 TriggerFlags; #define AFTER_TRIGGER_OFFSET 0x07FFFFFF /* must be low-order bits */ -#define AFTER_TRIGGER_DONE 0x10000000 -#define AFTER_TRIGGER_IN_PROGRESS 0x20000000 +#define AFTER_TRIGGER_DONE 0x80000000 +#define AFTER_TRIGGER_IN_PROGRESS 0x40000000 /* bits describing the size and tuple sources of this event */ #define AFTER_TRIGGER_FDW_REUSE 0x00000000 -#define AFTER_TRIGGER_FDW_FETCH 0x80000000 -#define AFTER_TRIGGER_1CTID 0x40000000 -#define AFTER_TRIGGER_2CTID 0xC0000000 +#define AFTER_TRIGGER_FDW_FETCH 0x20000000 +#define AFTER_TRIGGER_1CTID 0x10000000 +#define AFTER_TRIGGER_2CTID 0x30000000 #define AFTER_TRIGGER_CP_UPDATE 0x08000000 -#define AFTER_TRIGGER_TUP_BITS 0xC8000000 - +#define AFTER_TRIGGER_TUP_BITS 0x38000000 typedef struct AfterTriggerSharedData *AfterTriggerShared; typedef struct AfterTriggerSharedData @@ -3598,19 +3602,19 @@ typedef struct AfterTriggerEventData /* * During a cross-partition update of a partitioned table, we also store - * the OIDs of source and destination partitions that are needed to - * fetch the old (ctid1) and the new tuple (ctid2) from, respectively. + * the OIDs of source and destination partitions that are needed to fetch + * the old (ctid1) and the new tuple (ctid2) from, respectively. */ - Oid ate_src_part; - Oid ate_dst_part; + Oid ate_src_part; + Oid ate_dst_part; } AfterTriggerEventData; /* AfterTriggerEventData, minus ate_src_part, ate_dst_part */ typedef struct AfterTriggerEventDataNoOids { - TriggerFlags ate_flags; /* status bits and offset to shared data */ - ItemPointerData ate_ctid1; /* inserted, deleted, or old updated tuple */ - ItemPointerData ate_ctid2; /* new updated tuple */ + TriggerFlags ate_flags; + ItemPointerData ate_ctid1; + ItemPointerData ate_ctid2; } AfterTriggerEventDataNoOids; /* AfterTriggerEventData, minus ate_ctid2 */ @@ -4265,9 +4269,8 @@ AfterTriggerExecute(EState *estate, elog(ERROR, "failed to fetch tuple1 for AFTER trigger"); /* - * Store the tuple fetched from the source partition into - * the target (root partitioned) table slot, converting if - * needed. + * Store the tuple fetched from the source partition into the + * target (root partitioned) table slot, converting if needed. */ if (src_relInfo != relInfo) { @@ -4294,10 +4297,8 @@ AfterTriggerExecute(EState *estate, } /* don't touch ctid2 if not there */ - if (((event->ate_flags & AFTER_TRIGGER_TUP_BITS) == - AFTER_TRIGGER_2CTID || - (event->ate_flags & AFTER_TRIGGER_TUP_BITS) == - AFTER_TRIGGER_CP_UPDATE) && + if (((event->ate_flags & AFTER_TRIGGER_TUP_BITS) == AFTER_TRIGGER_2CTID || + (event->ate_flags & AFTER_TRIGGER_CP_UPDATE)) && ItemPointerIsValid(&(event->ate_ctid2))) { TupleTableSlot *dst_slot = ExecGetTriggerNewSlot(estate, @@ -4559,7 +4560,8 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, evtshared->ats_firing_id == firing_id) { ResultRelInfo *src_rInfo, - *dst_rInfo; + *dst_rInfo; + /* * So let's fire it... but first, find the correct relation if * this is not the same relation as before. @@ -4596,17 +4598,17 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, * Look up source and destination partition result rels of a * cross-partition update event. */ - if ((event->ate_flags & AFTER_TRIGGER_TUP_BITS ) == + if ((event->ate_flags & AFTER_TRIGGER_TUP_BITS) == AFTER_TRIGGER_CP_UPDATE) { Assert(OidIsValid(event->ate_src_part) && OidIsValid(event->ate_dst_part)); src_rInfo = ExecGetTriggerResultRel(estate, - event->ate_src_part, - rInfo); + event->ate_src_part, + rInfo); dst_rInfo = ExecGetTriggerResultRel(estate, - event->ate_dst_part, - rInfo); + event->ate_dst_part, + rInfo); } else src_rInfo = dst_rInfo = rInfo; @@ -5824,7 +5826,9 @@ AfterTriggerPendingOnRel(Oid relid) * as DELETE on the source partition followed by INSERT into the destination * partition. Specifically, firing DELETE triggers would lead to the wrong * foreign key action to be enforced considering that the original command is - * UPDATE. + * UPDATE; in this case, this function is called with relinfo as the + * partitioned table, and src_partinfo and dst_partinfo referring to the + * source and target leaf partitions, respectively. * ---------- */ static void @@ -5838,16 +5842,11 @@ AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, TransitionCaptureState *transition_capture) { Relation rel = relinfo->ri_RelationDesc; - Relation rootRel = relinfo->ri_RootResultRelInfo ? - relinfo->ri_RootResultRelInfo->ri_RelationDesc: NULL; - bool maybe_crosspart_update = - (row_trigger && mtstate && mtstate->operation == CMD_UPDATE && - (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE || - (rootRel && rootRel->rd_rel->relkind == - RELKIND_PARTITIONED_TABLE))); + Relation rootRel; TriggerDesc *trigdesc = relinfo->ri_TrigDesc; AfterTriggerEventData new_event; AfterTriggerSharedData new_shared; + bool maybe_crosspart_update; char relkind = rel->rd_rel->relkind; int tgtype_event; int tgtype_level; @@ -5953,11 +5952,18 @@ AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, /* * We normally don't see partitioned tables here for row level triggers - * except in the special case of a cross-partitioned update. In that - * case, nodeModifyTable.c: ExecCrossPartitionUpdateForeignKey() calls to + * except in the special case of a cross-partition update. In that case, + * nodeModifyTable.c:ExecCrossPartitionUpdateForeignKey() calls here to * queue an update event on the root target partitioned table, also * passing the source and destination partitions and their tuples. */ + rootRel = relinfo->ri_RootResultRelInfo ? + relinfo->ri_RootResultRelInfo->ri_RelationDesc : NULL; + maybe_crosspart_update = + (row_trigger && mtstate && + mtstate->operation == CMD_UPDATE && + (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE || + (rootRel && rootRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE))); Assert(!row_trigger || rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE || (maybe_crosspart_update && @@ -6024,6 +6030,7 @@ AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, Assert(newslot != NULL); ItemPointerCopy(&(oldslot->tts_tid), &(new_event.ate_ctid1)); ItemPointerCopy(&(newslot->tts_tid), &(new_event.ate_ctid2)); + /* * Also remember the OIDs of partitions to fetch these tuples * out of later in AfterTriggerExecute(). @@ -6031,8 +6038,10 @@ AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { Assert(src_partinfo != NULL && dst_partinfo != NULL); - new_event.ate_src_part = RelationGetRelid(src_partinfo->ri_RelationDesc); - new_event.ate_dst_part = RelationGetRelid(dst_partinfo->ri_RelationDesc); + new_event.ate_src_part = + RelationGetRelid(src_partinfo->ri_RelationDesc); + new_event.ate_dst_part = + RelationGetRelid(dst_partinfo->ri_RelationDesc); } } else @@ -6058,11 +6067,19 @@ AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, break; } + /* Determine flags */ if (!(relkind == RELKIND_FOREIGN_TABLE && row_trigger)) - new_event.ate_flags = (row_trigger && event == TRIGGER_EVENT_UPDATE) ? - (relkind == RELKIND_PARTITIONED_TABLE ? AFTER_TRIGGER_CP_UPDATE : - AFTER_TRIGGER_2CTID) : - AFTER_TRIGGER_1CTID; + { + if (row_trigger && event == TRIGGER_EVENT_UPDATE) + { + if (relkind == RELKIND_PARTITIONED_TABLE) + new_event.ate_flags = AFTER_TRIGGER_CP_UPDATE; + else + new_event.ate_flags = AFTER_TRIGGER_2CTID; + } + else + new_event.ate_flags = AFTER_TRIGGER_1CTID; + } /* else, we'll initialize ate_flags for each trigger */ @@ -6070,8 +6087,8 @@ AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, /* * Must convert/copy the source and destination partition tuples into the - * root partitioned table's format/slot, because the processing in the loop - * below expects both oldslot and newslot tuples to be in that form. + * root partitioned table's format/slot, because the processing in the + * loop below expects both oldslot and newslot tuples to be in that form. */ if (row_trigger && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { @@ -6081,18 +6098,18 @@ AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, rootslot = ExecGetTriggerOldSlot(estate, relinfo); map = ExecGetChildToRootMap(src_partinfo); if (map) - oldslot = execute_attr_map_slot(map->attrMap, - oldslot, - rootslot); + oldslot = execute_attr_map_slot(map->attrMap, + oldslot, + rootslot); else oldslot = ExecCopySlot(rootslot, oldslot); rootslot = ExecGetTriggerNewSlot(estate, relinfo); map = ExecGetChildToRootMap(dst_partinfo); if (map) - newslot = execute_attr_map_slot(map->attrMap, - newslot, - rootslot); + newslot = execute_attr_map_slot(map->attrMap, + newslot, + rootslot); else newslot = ExecCopySlot(rootslot, newslot); } @@ -6125,23 +6142,24 @@ AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, /* * If the trigger is a foreign key enforcement trigger, there are * certain cases where we can skip queueing the event because we can - * tell by inspection that the FK constraint will still pass. - * There are also some cases during cross-partition updates of a - * partitioned table where queuing the event can be skipped. + * tell by inspection that the FK constraint will still pass. There + * are also some cases during cross-partition updates of a partitioned + * table where queuing the event can be skipped. */ if (TRIGGER_FIRED_BY_UPDATE(event) || TRIGGER_FIRED_BY_DELETE(event)) { switch (RI_FKey_trigger_type(trigger->tgfoid)) { case RI_TRIGGER_PK: + /* * For cross-partitioned updates of partitioned PK table, * skip the event fired by the component delete on the * source leaf partition unless the constraint originates - * in the partition itself (!tgisclone), because the update - * event that will be fired on the root (partitioned) - * target table will be used to perform the necessary - * foreign key enforcement action. + * in the partition itself (!tgisclone), because the + * update event that will be fired on the root + * (partitioned) target table will be used to perform the + * necessary foreign key enforcement action. */ if (maybe_crosspart_update && TRIGGER_FIRED_BY_DELETE(event) && @@ -6158,6 +6176,7 @@ AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, break; case RI_TRIGGER_FK: + /* * Update on trigger's FK table. We can skip the update * event fired on a partitioned table during a @@ -6179,12 +6198,14 @@ AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, break; case RI_TRIGGER_NONE: + /* * Not an FK trigger. No need to queue the update event - * fired during a cross-partitioned update of a partitioned - * table, because the same row trigger must be present in - * the leaf partition(s) that are affected as part of this - * update and the events fired on them are queued instead. + * fired during a cross-partitioned update of a + * partitioned table, because the same row trigger must be + * present in the leaf partition(s) that are affected as + * part of this update and the events fired on them are + * queued instead. */ if (row_trigger && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index b6a2d4f708..880b135058 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1448,7 +1448,7 @@ ExecCloseResultRelations(EState *estate) foreach(l, estate->es_opened_result_relations) { ResultRelInfo *resultRelInfo = lfirst(l); - ListCell *lc; + ListCell *lc; ExecCloseIndices(resultRelInfo); foreach(lc, resultRelInfo->ri_ancestorResultRels) diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 7d32630030..6a16d0e673 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -585,6 +585,9 @@ ExecGetUpdateNewTuple(ResultRelInfo *relinfo, * to access "junk" columns that are not going to be stored. * * Returns RETURNING result if any, otherwise NULL. + * *inserted_tuple is the tuple that's effectively inserted; + * *inserted_destrel is the relation where it was inserted. + * These are only set on success. FIXME -- see what happens on the "do nothing" cases. * * This may change the currently active tuple conversion map in * mtstate->mt_transition_capture, so the callers must take care to @@ -1433,7 +1436,7 @@ ldelete:; * for the caller. * * False is returned if the tuple we're trying to move is found to have been - * concurrently updated. In that case, the caller must to check if the + * concurrently updated. In that case, the caller must check if the * updated tuple that's returned in *retry_slot still needs to be re-routed, * and call this function again or perform a regular update accordingly. */ @@ -1598,16 +1601,16 @@ GetAncestorResultRels(ResultRelInfo *resultRelInfo) if (!partRel->rd_rel->relispartition) elog(ERROR, "cannot find ancestors of a non-partition result relation"); Assert(rootRelInfo != NULL); - rootRelOid = RelationGetRelid(rootRelInfo->ri_RelationDesc); + rootRelOid = RelationGetRelid(rootRelInfo->ri_RelationDesc); if (resultRelInfo->ri_ancestorResultRels == NIL) { - ListCell *lc; - List *oids = get_partition_ancestors(RelationGetRelid(partRel)); - List *ancResultRels = NIL; + ListCell *lc; + List *oids = get_partition_ancestors(RelationGetRelid(partRel)); + List *ancResultRels = NIL; foreach(lc, oids) { - Oid ancOid = lfirst_oid(lc); + Oid ancOid = lfirst_oid(lc); Relation ancRel; ResultRelInfo *rInfo; @@ -1634,9 +1637,9 @@ GetAncestorResultRels(ResultRelInfo *resultRelInfo) } /* - * Queues up an update event using the target root partitioned table's trigger - * to check that a cross-partition update hasn't broken any foreign keys - * pointing into it. + * Queues up an update event using the target root partitioned table's + * trigger to check that a cross-partition update hasn't broken any foreign + * keys pointing into it. */ static void ExecCrossPartitionUpdateForeignKey(ResultRelInfo *sourcePartInfo, @@ -1647,9 +1650,9 @@ ExecCrossPartitionUpdateForeignKey(ResultRelInfo *sourcePartInfo, ModifyTableState *mtstate, EState *estate) { - ListCell *lc; + ListCell *lc; ResultRelInfo *rootRelInfo = sourcePartInfo->ri_RootResultRelInfo; - List *ancestorRels = GetAncestorResultRels(sourcePartInfo); + List *ancestorRels = GetAncestorResultRels(sourcePartInfo); /* * For any foreign keys that point directly into a non-root ancestors of @@ -1662,18 +1665,18 @@ ExecCrossPartitionUpdateForeignKey(ResultRelInfo *sourcePartInfo, { ResultRelInfo *rInfo = lfirst(lc); TriggerDesc *trigdesc = rInfo->ri_TrigDesc; - bool has_noncloned_fkey = false; + bool has_noncloned_fkey = false; if (rInfo == rootRelInfo) break; if (trigdesc && trigdesc->trig_update_after_row) { - int i; + int i; for (i = 0; i < trigdesc->numtriggers; i++) { - Trigger *trig = &trigdesc->triggers[i]; + Trigger *trig = &trigdesc->triggers[i]; if (!trig->tgisclone && RI_FKey_trigger_type(trig->tgfoid) == RI_TRIGGER_PK) @@ -1692,7 +1695,7 @@ ExecCrossPartitionUpdateForeignKey(ResultRelInfo *sourcePartInfo, RelationGetRelationName(rInfo->ri_RelationDesc), RelationGetRelationName(rootRelInfo->ri_RelationDesc)), errhint("Consider defining the foreign key on \"%s\".", - RelationGetRelationName(rootRelInfo->ri_RelationDesc)))); + RelationGetRelationName(rootRelInfo->ri_RelationDesc)))); } /* Perform the root table's triggers. */ @@ -1903,13 +1906,13 @@ lreplace:; /* * If the partitioned table being updated is referenced in foreign * keys, queue up trigger events to check that none of them were - * violated. No special treatment is needed in non-cross-partition - * update situations, because the leaf partition's AR update - * triggers will take care of that. During cross-partition - * updates implemented as delete on the source partition followed - * by insert on the destination partition, AR update triggers of - * the root table (that is, the table mentioned in the query) must - * be fired. + * violated. No special treatment is needed in + * non-cross-partition update situations, because the leaf + * partition's AR update triggers will take care of that. During + * cross-partition updates implemented as delete on the source + * partition followed by insert on the destination partition, + * AR-UPDATE triggers of the root table (that is, the table + * mentioned in the query) must be fired. * * NULL insert_destrel means that the move failed to occur, that * is, the update failed, so no need to anything in that case. diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index dcff6aeca5..03c587730c 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -204,7 +204,7 @@ extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, ResultRelInfo *partition_root_rri, int instrument_options); extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid, - ResultRelInfo *rootRelInfo); + ResultRelInfo *rootRelInfo); extern void ExecConstraints(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); extern bool ExecPartitionCheck(ResultRelInfo *resultRelInfo, -- 2.30.2