From 484a29e63ff0935aaa50375012fee93f9839c439 Mon Sep 17 00:00:00 2001 From: amitlan Date: Fri, 13 Nov 2020 18:24:48 +0900 Subject: [PATCH v9 2/2] Enforce foreign key correctly during cross-partition updates When an update on a partitioned table referenced in foreign keys constraint causes a row to move from one partition to another, which is implemented by deleting the old row from the source leaf partition followed by inserting the new row into the destination leaf partition, firing of the delete triggers that implement those foreign keys can result in surprising outcomes for those keys. For example, a given foreign key's delete trigger which implements the ON DELETE CASCADE clause of that key will delete any referencing rows, although it should not, because the referenced row is simply being moved into another partition. This commit teaches trigger.c to skip queuing such delete trigger events on the leaf partitions in favor of an UPDATE event fired on the "root" target relation. Doing so makes sense because both the old and the new tuple "logically" belong to the latter. To make this possible, this adjusts AFTER trigger data strucutures to allow queuing and firing events containing partitioned table's tuples. Given that partitioned tables are only logical relations, meaning that its tuples have no physical identifiers, the only way to remember the event tuples seems to be to store them in a tuplestore, similar to what is currently done for foreign tables. Although, for partitioned tables, these tuplestores may need to outlive the query in which the trigger event was queued. For example, if the foreign key pointing to the partitioned table is marked INITIALLY DEFERRED, the tuples must be remembered untill transaction ends. That is ensured by allocating the tuplestore under TopTranscationContext and making TopTransactionResOwner its owner. The implementation currently has a limitation that only the foreign keys pointing into the query's target relation are considered, not those of its sub-partitioned partitions. That seems like a reasonable limitation, it sounds rare to have distinct foreign keys pointing into sub-partitioned partitions, but not into the root table. --- doc/src/sgml/ref/update.sgml | 7 + src/backend/commands/trigger.c | 305 +++++++++++++++++----- src/backend/executor/execMain.c | 9 + src/backend/executor/execReplication.c | 4 +- src/backend/executor/nodeModifyTable.c | 237 ++++++++++++++++- src/backend/utils/adt/ri_triggers.c | 17 +- src/include/commands/trigger.h | 2 + src/include/nodes/execnodes.h | 3 + src/test/regress/expected/foreign_key.out | 201 +++++++++++++- src/test/regress/sql/foreign_key.sql | 132 +++++++++- 10 files changed, 826 insertions(+), 91 deletions(-) diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml index 3fa54e5f70..3ba13010e7 100644 --- a/doc/src/sgml/ref/update.sgml +++ b/doc/src/sgml/ref/update.sgml @@ -316,6 +316,13 @@ UPDATE count partition (provided the foreign data wrapper supports tuple routing), they cannot be moved from a foreign-table partition to another partition. + + + An attempt of moving a row from one partition to another will fail if a + foreign key is found to directly reference a non-root partitioned table + in the partition tree, unless that table is also directly mentioned + in the UPDATEquery. + diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 6e97285a30..3b1fdefdd1 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -94,13 +94,18 @@ static HeapTuple ExecCallTriggerFunc(TriggerData *trigdata, FmgrInfo *finfo, Instrumentation *instr, MemoryContext per_tuple_context); -static void AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, +static void AfterTriggerSaveEvent(EState *estate, + ModifyTableState *mtstate, + ResultRelInfo *relinfo, int event, bool row_trigger, TupleTableSlot *oldtup, TupleTableSlot *newtup, List *recheckIndexes, Bitmapset *modifiedCols, TransitionCaptureState *transition_capture); static void AfterTriggerEnlargeQueryState(void); static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType); +static bool SkipCrossPartitionUpdateFKeyTrigger(ModifyTableState *mtstate, + Trigger *trigger, int event, + Relation rel); /* @@ -2456,7 +2461,7 @@ ExecASInsertTriggers(EState *estate, ResultRelInfo *relinfo, TriggerDesc *trigdesc = relinfo->ri_TrigDesc; if (trigdesc && trigdesc->trig_insert_after_statement) - AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_INSERT, + AfterTriggerSaveEvent(estate, NULL, relinfo, TRIGGER_EVENT_INSERT, false, NULL, NULL, NIL, NULL, transition_capture); } @@ -2545,7 +2550,7 @@ ExecARInsertTriggers(EState *estate, ResultRelInfo *relinfo, if ((trigdesc && trigdesc->trig_insert_after_row) || (transition_capture && transition_capture->tcs_insert_new_table)) - AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_INSERT, + AfterTriggerSaveEvent(estate, NULL, relinfo, TRIGGER_EVENT_INSERT, true, NULL, slot, recheckIndexes, NULL, transition_capture); @@ -2670,7 +2675,7 @@ ExecASDeleteTriggers(EState *estate, ResultRelInfo *relinfo, TriggerDesc *trigdesc = relinfo->ri_TrigDesc; if (trigdesc && trigdesc->trig_delete_after_statement) - AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_DELETE, + AfterTriggerSaveEvent(estate, NULL, relinfo, TRIGGER_EVENT_DELETE, false, NULL, NULL, NIL, NULL, transition_capture); } @@ -2767,7 +2772,8 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate, } void -ExecARDeleteTriggers(EState *estate, ResultRelInfo *relinfo, +ExecARDeleteTriggers(EState *estate, ModifyTableState *mtstate, + ResultRelInfo *relinfo, ItemPointer tupleid, HeapTuple fdw_trigtuple, TransitionCaptureState *transition_capture) @@ -2791,7 +2797,7 @@ ExecARDeleteTriggers(EState *estate, ResultRelInfo *relinfo, else ExecForceStoreHeapTuple(fdw_trigtuple, slot, false); - AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_DELETE, + AfterTriggerSaveEvent(estate, mtstate, relinfo, TRIGGER_EVENT_DELETE, true, slot, NULL, NIL, NULL, transition_capture); } @@ -2912,7 +2918,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo, Assert(relinfo->ri_RootResultRelInfo == NULL); if (trigdesc && trigdesc->trig_update_after_statement) - AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_UPDATE, + AfterTriggerSaveEvent(estate, NULL, relinfo, TRIGGER_EVENT_UPDATE, false, NULL, NULL, NIL, ExecGetAllUpdatedCols(relinfo, estate), transition_capture); @@ -3051,7 +3057,8 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, } void -ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo, +ExecARUpdateTriggers(EState *estate, ModifyTableState *mtstate, + ResultRelInfo *relinfo, ItemPointer tupleid, HeapTuple fdw_trigtuple, TupleTableSlot *newslot, @@ -3086,7 +3093,7 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo, else ExecClearTuple(oldslot); - AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_UPDATE, + AfterTriggerSaveEvent(estate, mtstate, relinfo, TRIGGER_EVENT_UPDATE, true, oldslot, newslot, recheckIndexes, ExecGetAllUpdatedCols(relinfo, estate), transition_capture); @@ -3212,7 +3219,7 @@ ExecASTruncateTriggers(EState *estate, ResultRelInfo *relinfo) TriggerDesc *trigdesc = relinfo->ri_TrigDesc; if (trigdesc && trigdesc->trig_truncate_after_statement) - AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_TRUNCATE, + AfterTriggerSaveEvent(estate, NULL, relinfo, TRIGGER_EVENT_TRUNCATE, false, NULL, NULL, NIL, NULL, NULL); } @@ -3500,19 +3507,23 @@ typedef SetConstraintStateData *SetConstraintState; * * 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 - * tables use one; foreign tables always use zero and save the tuple(s) to a - * tuplestore. AFTER_TRIGGER_FDW_FETCH directs AfterTriggerExecute() to - * retrieve a fresh tuple or pair of tuples from that tuplestore, while - * AFTER_TRIGGER_FDW_REUSE directs it to use the most-recently-retrieved - * tuple(s). This permits storing tuples once regardless of the number of - * row-level triggers on a foreign table. + * tables use one; foreign or partitioned tables always use zero and save the + * tuple(s) to a tuplestore. AFTER_TRIGGER_TS_FETCH directs + * AfterTriggerExecute() to retrieve a fresh tuple or pair of tuples from that + * tuplestore, while AFTER_TRIGGER_TS_REUSE directs it to use the + * most-recently-retrieved tuple(s). This permits storing tuples once + * regardless of the number of row-level triggers on a foreign or partitioned + * table. * - * 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) - * triggers on foreign tables. This also ensures that such triggers do not - * get deferred into outer trigger query levels, meaning that it's okay to - * destroy the tuplestore at the end of the query level. + * Note that we need triggers on foreign and partitioned 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) triggers on foreign tables. This also ensures that such + * triggers do not get deferred into outer trigger query levels, meaning that + * it's okay to destroy the tuplestore at the end of the query level. + * XXX - update this paragraph if the new approach, whereby tuplestores in + * afterTriggers.deferred_tuplestores outlive any given query, can be proven + * to not really break any assumptions mentioned here. * * Statement-level triggers always bear AFTER_TRIGGER_1CTID, though they * require no ctid field. We lack the flag bit space to neatly represent that @@ -3533,8 +3544,8 @@ typedef uint32 TriggerFlags; #define AFTER_TRIGGER_DONE 0x10000000 #define AFTER_TRIGGER_IN_PROGRESS 0x20000000 /* 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_TS_REUSE 0x00000000 +#define AFTER_TRIGGER_TS_FETCH 0x80000000 #define AFTER_TRIGGER_1CTID 0x40000000 #define AFTER_TRIGGER_2CTID 0xC0000000 #define AFTER_TRIGGER_TUP_BITS 0xC0000000 @@ -3549,6 +3560,8 @@ typedef struct AfterTriggerSharedData CommandId ats_firing_id; /* ID for firing cycle */ struct AfterTriggersTableData *ats_table; /* transition table access */ Bitmapset *ats_modifiedcols; /* modified columns */ + Tuplestorestate *tuplestore; /* set if relation is a foreign or + * a partitioned table */ } AfterTriggerSharedData; typedef struct AfterTriggerEventData *AfterTriggerEvent; @@ -3659,11 +3672,12 @@ typedef struct AfterTriggerEventList * occurs. At that point we fire immediate-mode triggers, and append any * deferred events to the main events list. * - * fdw_tuplestore is a tuplestore containing the foreign-table tuples - * needed by events queued by the current query. (Note: we use just one - * tuplestore even though more than one foreign table might be involved. - * This is okay because tuplestores don't really care what's in the tuples - * they store; but it's possible that someday it'd break.) + * tuplestore is a tuplestore containing the foreign or partitioned table + * tuples needed by events queued by the current query. (Note: we use just + * one tuplestore even though more than one foreign or partitioned table + * might be involved. This is okay because tuplestores don't really care + * what's in the tuples they store; but it's possible that someday it'd + * break.) * * tables is a List of AfterTriggersTableData structs for target tables * of the current query (see below). @@ -3703,10 +3717,16 @@ typedef struct AfterTriggerEventList * That's sufficient lifespan because we don't allow transition tables to be * used by deferrable triggers, so they only need to survive until * AfterTriggerEndQuery. + * + * tuplestores stored in 'deferred_tuplestores' live in + * TopTransactionContext and owned by TopTransactionResourceOwner. They are + * used to store the tuples of partitioned tables that are the targets of + * any deferred constraint triggers that get fired during the transaction. */ typedef struct AfterTriggersQueryData AfterTriggersQueryData; typedef struct AfterTriggersTransData AfterTriggersTransData; typedef struct AfterTriggersTableData AfterTriggersTableData; +typedef struct AfterTriggersTuplestoreData AfterTriggersTuplestoreData; typedef struct AfterTriggersData { @@ -3723,12 +3743,16 @@ typedef struct AfterTriggersData /* per-subtransaction-level data: */ AfterTriggersTransData *trans_stack; /* array of structs shown below */ int maxtransdepth; /* allocated len of above array */ + + /* transaction-lifetime data: */ + List *deferred_tuplestores; } AfterTriggersData; struct AfterTriggersQueryData { AfterTriggerEventList events; /* events pending from this query */ - Tuplestorestate *fdw_tuplestore; /* foreign tuples for said events */ + Tuplestorestate *tuplestore; /* foreign or partitioned table tuples for + * said events */ List *tables; /* list of AfterTriggersTableData, see below */ }; @@ -3757,6 +3781,18 @@ struct AfterTriggersTableData static AfterTriggersData afterTriggers; +/* + * For each partitioned tables whose deferrable constraint triggers get fired + * during the transaction. + * + * Instances of this are added to afterTriggers.deferred_tuplestores. + */ +struct AfterTriggersTuplestoreData +{ + Oid relid; /* target table's OID */ + Tuplestorestate *tuplestore; /* to store target table's tuples */ +}; + static void AfterTriggerExecute(EState *estate, AfterTriggerEvent event, ResultRelInfo *relInfo, @@ -3779,37 +3815,74 @@ static void cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent); /* - * Get the FDW tuplestore for the current trigger query level, creating it - * if necessary. + * Get the special tuplestore needed to store foreign table or partitioned + * table trigger tuples, creating it in the correct context if necessary. */ static Tuplestorestate * -GetCurrentFDWTuplestore(void) +GetAfterTriggersTuplestore(Relation rel, bool initdeferred) { - Tuplestorestate *ret; + MemoryContext oldcxt; + ResourceOwner saveResourceOwner; + Tuplestorestate **ret; - ret = afterTriggers.query_stack[afterTriggers.query_depth].fdw_tuplestore; - if (ret == NULL) + /* + * If the per-query tuplestore has been set, that's the one that must be + * used to all events triggered in this query. + */ + if (afterTriggers.query_stack[afterTriggers.query_depth].tuplestore) + return afterTriggers.query_stack[afterTriggers.query_depth].tuplestore; + + /* + * If the event must be remembered till transaction end, so must the + * tuplestore, so allocate under top-level transaction. + */ + if (initdeferred) { - MemoryContext oldcxt; - ResourceOwner saveResourceOwner; + ListCell *lc; + AfterTriggersTuplestoreData *tuplestore_data; + + /* + * It's okay to use the same tuplestore for a given relation OID even + * if the triggering query may have changed. + */ + foreach(lc, afterTriggers.deferred_tuplestores) + { + AfterTriggersTuplestoreData *tuplestore_data = + (AfterTriggersTuplestoreData *) lfirst(lc); + + if (tuplestore_data->relid == RelationGetRelid(rel)) + return tuplestore_data->tuplestore; + } + + oldcxt = MemoryContextSwitchTo(TopTransactionContext); + saveResourceOwner = CurrentResourceOwner; + CurrentResourceOwner = TopTransactionResourceOwner; + + tuplestore_data = palloc(sizeof(AfterTriggersTuplestoreData)); + afterTriggers.deferred_tuplestores = + lappend(afterTriggers.deferred_tuplestores, tuplestore_data); + tuplestore_data->relid = RelationGetRelid(rel); + ret = &tuplestore_data->tuplestore; + } + else + { + ret = &afterTriggers.query_stack[afterTriggers.query_depth].tuplestore; /* * Make the tuplestore valid until end of subtransaction. We really - * only need it until AfterTriggerEndQuery(). + * only need it until AfterTriggerEndQuery() though. */ oldcxt = MemoryContextSwitchTo(CurTransactionContext); saveResourceOwner = CurrentResourceOwner; CurrentResourceOwner = CurTransactionResourceOwner; + } - ret = tuplestore_begin_heap(false, false, work_mem); + *ret = tuplestore_begin_heap(false, false, work_mem); - CurrentResourceOwner = saveResourceOwner; - MemoryContextSwitchTo(oldcxt); - - afterTriggers.query_stack[afterTriggers.query_depth].fdw_tuplestore = ret; - } + CurrentResourceOwner = saveResourceOwner; + MemoryContextSwitchTo(oldcxt); - return ret; + return *ret; } /* ---------- @@ -4140,22 +4213,26 @@ AfterTriggerExecute(EState *estate, */ switch (event->ate_flags & AFTER_TRIGGER_TUP_BITS) { - case AFTER_TRIGGER_FDW_FETCH: + case AFTER_TRIGGER_TS_FETCH: { - Tuplestorestate *fdw_tuplestore = GetCurrentFDWTuplestore(); + Tuplestorestate *tuplestore; - if (!tuplestore_gettupleslot(fdw_tuplestore, true, false, + if (evtshared->tuplestore == NULL) + elog(ERROR, "no tuplestore to fetch tuples for AFTER trigger"); + tuplestore = evtshared->tuplestore; + + if (!tuplestore_gettupleslot(tuplestore, true, false, trig_tuple_slot1)) elog(ERROR, "failed to fetch tuple1 for AFTER trigger"); if ((evtshared->ats_event & TRIGGER_EVENT_OPMASK) == TRIGGER_EVENT_UPDATE && - !tuplestore_gettupleslot(fdw_tuplestore, true, false, + !tuplestore_gettupleslot(tuplestore, true, false, trig_tuple_slot2)) elog(ERROR, "failed to fetch tuple2 for AFTER trigger"); } /* fall through */ - case AFTER_TRIGGER_FDW_REUSE: + case AFTER_TRIGGER_TS_REUSE: /* * Store tuple in the slot so that tg_trigtuple does not reference @@ -4458,7 +4535,8 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, ExecDropSingleTupleTableSlot(slot2); slot1 = slot2 = NULL; } - if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) + if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE || + rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { slot1 = MakeSingleTupleTableSlot(rel->rd_att, &TTSOpsMinimalTuple); @@ -4734,6 +4812,7 @@ AfterTriggerBeginXact(void) Assert(afterTriggers.events.head == NULL); Assert(afterTriggers.trans_stack == NULL); Assert(afterTriggers.maxtransdepth == 0); + Assert(afterTriggers.deferred_tuplestores == NIL); } @@ -4867,8 +4946,8 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs) afterTriggerFreeEventList(&qs->events); /* Drop FDW tuplestore if any */ - ts = qs->fdw_tuplestore; - qs->fdw_tuplestore = NULL; + ts = qs->tuplestore; + qs->tuplestore = NULL; if (ts) tuplestore_end(ts); @@ -5005,6 +5084,7 @@ AfterTriggerEndXact(bool isCommit) afterTriggers.query_stack = NULL; afterTriggers.maxquerydepth = 0; afterTriggers.state = NULL; + afterTriggers.deferred_tuplestores = NIL; /* No more afterTriggers manipulation until next transaction starts. */ afterTriggers.query_depth = -1; @@ -5202,7 +5282,7 @@ AfterTriggerEnlargeQueryState(void) qs->events.head = NULL; qs->events.tail = NULL; qs->events.tailfree = NULL; - qs->fdw_tuplestore = NULL; + qs->tuplestore = NULL; qs->tables = NIL; ++init_depth; @@ -5673,7 +5753,8 @@ AfterTriggerPendingOnRel(Oid relid) * ---------- */ static void -AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, +AfterTriggerSaveEvent(EState *estate, ModifyTableState *mtstate, + ResultRelInfo *relinfo, int event, bool row_trigger, TupleTableSlot *oldslot, TupleTableSlot *newslot, List *recheckIndexes, Bitmapset *modifiedCols, @@ -5687,7 +5768,8 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, int tgtype_event; int tgtype_level; int i; - Tuplestorestate *fdw_tuplestore = NULL; + Tuplestorestate *immediate_tuplestore = NULL; + Tuplestorestate *deferred_tuplestore = NULL; /* * Check state. We use a normal test not Assert because it is possible to @@ -5870,7 +5952,9 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, break; } - if (!(relkind == RELKIND_FOREIGN_TABLE && row_trigger)) + if (!row_trigger || + (relkind != RELKIND_FOREIGN_TABLE && + relkind != RELKIND_PARTITIONED_TABLE)) new_event.ate_flags = (row_trigger && event == TRIGGER_EVENT_UPDATE) ? AFTER_TRIGGER_2CTID : AFTER_TRIGGER_1CTID; /* else, we'll initialize ate_flags for each trigger */ @@ -5880,6 +5964,7 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, for (i = 0; i < trigdesc->numtriggers; i++) { Trigger *trigger = &trigdesc->triggers[i]; + Tuplestorestate *tuplestore = NULL; if (!TRIGGER_TYPE_MATCHES(trigger->tgtype, tgtype_level, @@ -5890,16 +5975,37 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, modifiedCols, oldslot, newslot)) continue; - if (relkind == RELKIND_FOREIGN_TABLE && row_trigger) + if (mtstate && mtstate->operation == CMD_UPDATE && + SkipCrossPartitionUpdateFKeyTrigger(mtstate, trigger, event, rel)) + continue; + + if (row_trigger && + (relkind == RELKIND_FOREIGN_TABLE || + relkind == RELKIND_PARTITIONED_TABLE)) { - if (fdw_tuplestore == NULL) + bool first; + + if (trigger->tginitdeferred) + { + first = (deferred_tuplestore == NULL); + deferred_tuplestore = tuplestore = + GetAfterTriggersTuplestore(rel, true); + } + else + { + first = (immediate_tuplestore == NULL); + immediate_tuplestore = tuplestore = + GetAfterTriggersTuplestore(rel, false); + } + + if (first) { - fdw_tuplestore = GetCurrentFDWTuplestore(); - new_event.ate_flags = AFTER_TRIGGER_FDW_FETCH; + new_event.ate_flags = AFTER_TRIGGER_TS_FETCH; + first = false; } else /* subsequent event for the same tuple */ - new_event.ate_flags = AFTER_TRIGGER_FDW_REUSE; + new_event.ate_flags = AFTER_TRIGGER_TS_REUSE; } /* @@ -5967,23 +6073,84 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, else new_shared.ats_table = NULL; new_shared.ats_modifiedcols = modifiedCols; + new_shared.tuplestore = tuplestore; afterTriggerAddEvent(&afterTriggers.query_stack[afterTriggers.query_depth].events, &new_event, &new_shared); } /* - * Finally, spool any foreign tuple(s). The tuplestore squashes them to - * minimal tuples, so this loses any system columns. The executor lost - * those columns before us, for an unrelated reason, so this is fine. + * Finally, spool any foreign or partitioned table tuple(s). The + * tuplestore squashes them to minimal tuples, so this loses any system + * columns. The executor lost those columns before us, for an unrelated + * reason, so this is fine. */ - if (fdw_tuplestore) + if (immediate_tuplestore) + { + if (oldslot != NULL) + tuplestore_puttupleslot(immediate_tuplestore, oldslot); + if (newslot != NULL) + tuplestore_puttupleslot(immediate_tuplestore, newslot); + } + if (deferred_tuplestore) { if (oldslot != NULL) - tuplestore_puttupleslot(fdw_tuplestore, oldslot); + tuplestore_puttupleslot(deferred_tuplestore, oldslot); if (newslot != NULL) - tuplestore_puttupleslot(fdw_tuplestore, newslot); + tuplestore_puttupleslot(deferred_tuplestore, newslot); + } +} + +/* + * Some events fired during the UPDATEs of partitioned tables that + * are turned into DELETE+INSERT must be skipped. + */ +static bool +SkipCrossPartitionUpdateFKeyTrigger(ModifyTableState *mtstate, + Trigger *trigger, int event, + Relation rel) +{ + Relation rootRelDesc = mtstate->rootResultRelInfo->ri_RelationDesc; + + if (rootRelDesc->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) + return false; + + switch (RI_FKey_trigger_type(trigger->tgfoid)) + { + /* + * For UPDATEs of partitioned PK table, skip the events fired + * by the DELETEs unless the constraint originates in the + * relation on which it is fired (!tgisclone), because the + * UPDATE event fired on the root (partitioned) target table + * will be queued instead. + */ + case RI_TRIGGER_PK: + if (TRIGGER_FIRED_BY_DELETE(event) && trigger->tgisclone) + return true; + break; + + /* + * Skip events on the root partitione table if: 1) it's the FK + * table, because the events fired on the destination leaf + * partition suffice to do the checks necessary to enforce + * the FK relationship, 2) the trigger is unrelated to foreign + * keys, because the instance of the trigger in the leaf + * partitions will be fired instead. In fact, proceeding with + * firing the event on the partitioned table can be unsafe in + * both cases. For (1), RI_FKey_check() can't handle being + * handed a partitioned table. For (2), the trigger may be + * a INITIALLY DEFERRED constraint trigger, for which we + * can't ensure the event's tuples will be accessible when + * the trigger is fired. + */ + case RI_TRIGGER_FK: + case RI_TRIGGER_NONE: + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + return true; + break; } + + return false; } /* diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index b3ce4bae53..f7ce03bc14 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1447,8 +1447,17 @@ ExecCloseResultRelations(EState *estate) foreach(l, estate->es_opened_result_relations) { ResultRelInfo *resultRelInfo = lfirst(l); + ListCell *lc; ExecCloseIndices(resultRelInfo); + foreach(lc, resultRelInfo->ri_ancestorResultRels) + { + ResultRelInfo *rInfo = lfirst(lc); + + /* Only close those we opened in GetAncestorResultRels(). */ + if (rInfo->ri_RangeTableIndex == 0) + table_close(rInfo->ri_RelationDesc, NoLock); + } } /* Close any relations that have been opened by ExecGetTriggerResultRel(). */ diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 574d7d27fd..747347b5bf 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -516,7 +516,7 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo, NULL, NIL); /* AFTER ROW UPDATE Triggers */ - ExecARUpdateTriggers(estate, resultRelInfo, + ExecARUpdateTriggers(estate, NULL, resultRelInfo, tid, NULL, slot, recheckIndexes, NULL); @@ -556,7 +556,7 @@ ExecSimpleRelationDelete(ResultRelInfo *resultRelInfo, simple_table_tuple_delete(rel, tid, estate->es_snapshot); /* AFTER ROW DELETE Triggers */ - ExecARDeleteTriggers(estate, resultRelInfo, + ExecARDeleteTriggers(estate, NULL, resultRelInfo, tid, NULL, NULL); } } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index d328856ae5..84e99b2f3b 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -38,6 +38,7 @@ #include "access/tableam.h" #include "access/xact.h" #include "catalog/catalog.h" +#include "catalog/partition.h" #include "commands/trigger.h" #include "executor/execPartition.h" #include "executor/executor.h" @@ -596,7 +597,9 @@ ExecInsert(ModifyTableState *mtstate, TupleTableSlot *slot, TupleTableSlot *planSlot, EState *estate, - bool canSetTag) + bool canSetTag, + TupleTableSlot **inserted_tuple, + ResultRelInfo **insert_destrel) { Relation resultRelationDesc; List *recheckIndexes = NIL; @@ -956,7 +959,7 @@ ExecInsert(ModifyTableState *mtstate, if (mtstate->operation == CMD_UPDATE && mtstate->mt_transition_capture && mtstate->mt_transition_capture->tcs_update_new_table) { - ExecARUpdateTriggers(estate, resultRelInfo, NULL, + ExecARUpdateTriggers(estate, mtstate, resultRelInfo, NULL, NULL, slot, NULL, @@ -994,6 +997,11 @@ ExecInsert(ModifyTableState *mtstate, if (resultRelInfo->ri_projectReturning) result = ExecProcessReturning(resultRelInfo, slot, planSlot); + if (inserted_tuple) + *inserted_tuple = slot; + if (insert_destrel) + *insert_destrel = resultRelInfo; + return result; } @@ -1346,7 +1354,7 @@ ldelete:; if (mtstate->operation == CMD_UPDATE && mtstate->mt_transition_capture && mtstate->mt_transition_capture->tcs_update_old_table) { - ExecARUpdateTriggers(estate, resultRelInfo, + ExecARUpdateTriggers(estate, mtstate, resultRelInfo, tupleid, oldtuple, NULL, @@ -1361,7 +1369,7 @@ ldelete:; } /* AFTER ROW DELETE Triggers */ - ExecARDeleteTriggers(estate, resultRelInfo, tupleid, oldtuple, + ExecARDeleteTriggers(estate, mtstate, resultRelInfo, tupleid, oldtuple, ar_delete_trig_tcs); /* Process RETURNING if present and if requested */ @@ -1433,7 +1441,9 @@ ExecCrossPartitionUpdate(ModifyTableState *mtstate, TupleTableSlot *slot, TupleTableSlot *planSlot, EPQState *epqstate, bool canSetTag, TupleTableSlot **retry_slot, - TupleTableSlot **inserted_tuple) + TupleTableSlot **returning_slot, + TupleTableSlot **inserted_tuple, + ResultRelInfo **insert_destrel) { EState *estate = mtstate->ps.state; TupleConversionMap *tupconv_map; @@ -1556,8 +1566,9 @@ ExecCrossPartitionUpdate(ModifyTableState *mtstate, mtstate->mt_root_tuple_slot); /* Tuple routing starts from the root table. */ - *inserted_tuple = ExecInsert(mtstate, mtstate->rootResultRelInfo, slot, - planSlot, estate, canSetTag); + *returning_slot = ExecInsert(mtstate, mtstate->rootResultRelInfo, slot, + planSlot, estate, canSetTag, inserted_tuple, + insert_destrel); /* * Reset the transition state that may possibly have been written by @@ -1570,6 +1581,180 @@ ExecCrossPartitionUpdate(ModifyTableState *mtstate, return true; } +/* + * Returns tuple table slot that the caller can use to store the tuples in the + * the root target relation's format, creating it if not already done. + */ +static TupleTableSlot * +GetRootTupleSlot(ModifyTableState *mtstate) +{ + if (mtstate->mt_root_tuple_slot == NULL) + { + Relation rootrel = mtstate->rootResultRelInfo->ri_RelationDesc; + + mtstate->mt_root_tuple_slot = table_slot_create(rootrel, NULL); + } + + return mtstate->mt_root_tuple_slot; +} + +/* + * Returns a map to convert the tuples of a given leaf partition result + * relation into the tuples of the root target relation, creating it if not + * already done. + */ +static TupleConversionMap * +GetChildToRootMap(ResultRelInfo *resultRelInfo, ModifyTableState *mtstate) +{ + if (!resultRelInfo->ri_ChildToRootMapValid) + { + Relation relation = resultRelInfo->ri_RelationDesc; + Relation rootRel = mtstate->rootResultRelInfo->ri_RelationDesc; + + resultRelInfo->ri_ChildToRootMap = + convert_tuples_by_name(RelationGetDescr(relation), + RelationGetDescr(rootRel)); + resultRelInfo->ri_ChildToRootMapValid = true; + } + + return resultRelInfo->ri_ChildToRootMap; +} + +/* + * Return the ancestor relations of a given leaf partition result relation + * up to and including the query's root target relation. + */ +static List * +GetAncestorResultRels(ResultRelInfo *resultRelInfo, + ModifyTableState *mtstate) +{ + Relation partRel = resultRelInfo->ri_RelationDesc; + Relation rootRel = mtstate->rootResultRelInfo->ri_RelationDesc; + + if (!partRel->rd_rel->relispartition) + elog(ERROR, "cannot find ancestors of a non-partition result relation"); + if (resultRelInfo->ri_ancestorResultRels == NIL) + { + ListCell *lc; + List *oids = get_partition_ancestors(RelationGetRelid(partRel)); + Oid rootRelOid = RelationGetRelid(rootRel); + List *ancResultRels = NIL; + + foreach(lc, oids) + { + Oid ancOid = lfirst_oid(lc); + Relation ancRel; + ResultRelInfo *rInfo; + + /* We use mtstate->rootResultRelInfo for the root relation. */ + if (ancOid == rootRelOid) + break; + + /* + * All ancestors up to the root target relation must have been + * locked by the planner or AcquireExecutorLocks(). + */ + ancRel = table_open(ancOid, NoLock); + rInfo = makeNode(ResultRelInfo); + + /* + * Pass 0 for RangeTableIndex to distinguish the relations that + * are opened here. + */ + InitResultRelInfo(rInfo, ancRel, 0, NULL, 0); + ancResultRels = lappend(ancResultRels, rInfo); + } + ancResultRels = lappend(ancResultRels, mtstate->rootResultRelInfo); + resultRelInfo->ri_ancestorResultRels = ancResultRels; + } + + return resultRelInfo->ri_ancestorResultRels; +} + +/* + * Queues up trigger events necessary to check that a cross-partition update + * of the target partitioned table hasn't broken any foreign keys pointing + * to it. + */ +static void +ExecCrossPartitionUpdateForeignKey(ResultRelInfo *sourcePartInfo, + ItemPointer tupleid, + TupleTableSlot *oldslot, + TupleTableSlot *newslot, + ModifyTableState *mtstate, + EState *estate) +{ + ListCell *lc; + HeapTuple oldtuple; + TupleTableSlot *rootslot; + TupleConversionMap *map; + ResultRelInfo *rootInfo = mtstate->rootResultRelInfo; + Relation sourcePartRelDesc = sourcePartInfo->ri_RelationDesc; + List *ancestorRels = GetAncestorResultRels(sourcePartInfo, mtstate); + + /* + * There better not be any foreign keys that point directly to a non-root + * ancestor of the target source partition, because we can't enforce them. + */ + foreach(lc, ancestorRels) + { + ResultRelInfo *rInfo = lfirst(lc); + TriggerDesc *trigdesc = rInfo->ri_TrigDesc; + bool has_noncloned_fkey = false; + + if (rInfo == mtstate->rootResultRelInfo) + break; + + if (trigdesc && trigdesc->trig_update_after_row) + { + int i; + + for (i = 0; i < trigdesc->numtriggers; i++) + { + Trigger *trig = &trigdesc->triggers[i]; + + if (!trig->tgisclone && + RI_FKey_trigger_type(trig->tgfoid) == RI_TRIGGER_PK) + { + has_noncloned_fkey = true; + break; + } + } + } + + if (has_noncloned_fkey) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot move tuple across partitions when non-root ancestor is directly referenced in a foreign key"), + errdetail("A foreign key points to ancestor \"%s\", but not the root ancestor \"%s\".", + RelationGetRelationName(rInfo->ri_RelationDesc), + RelationGetRelationName(rootInfo->ri_RelationDesc)), + errhint("Consider defining the foreign key on \"%s\".", + RelationGetRelationName(rootInfo->ri_RelationDesc)))); + } + + /* + * Copy the inserted "new" tuple into the root table's slot, after + * converting it if needed. + */ + rootslot = GetRootTupleSlot(mtstate); + map = GetChildToRootMap(sourcePartInfo, mtstate); + if (newslot != oldslot && map) + newslot = execute_attr_map_slot(map->attrMap, newslot, rootslot); + else + newslot = ExecCopySlot(rootslot, newslot); + + /* Get "old" HeapTuple from the source partition. */ + if (!table_tuple_fetch_row_version(sourcePartRelDesc, tupleid, + SnapshotAny, oldslot)) + elog(ERROR, "failed to fetch old tuple from source partition"); + oldtuple = ExecFetchSlotHeapTuple(oldslot, true, NULL); + + /* Perform the root table's triggers. */ + ExecARUpdateTriggers(estate, mtstate, rootInfo, NULL, oldtuple, + newslot, NIL, NULL); +} + /* ---------------------------------------------------------------- * ExecUpdate * @@ -1742,9 +1927,12 @@ lreplace:; */ if (partition_constraint_failed) { - TupleTableSlot *inserted_tuple, + TupleTableSlot *oldslot = slot, + *inserted_tuple, + *returning_slot = NULL, *retry_slot; bool retry; + ResultRelInfo *insert_destrel = NULL; /* * ExecCrossPartitionUpdate will first DELETE the row from the @@ -1756,14 +1944,38 @@ lreplace:; retry = !ExecCrossPartitionUpdate(mtstate, resultRelInfo, tupleid, oldtuple, slot, planSlot, epqstate, canSetTag, - &retry_slot, &inserted_tuple); + &retry_slot, &returning_slot, + &inserted_tuple, + &insert_destrel); if (retry) { slot = retry_slot; goto lreplace; } - return inserted_tuple; + /* + * 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. + * + * NULL insert_destrel means that the move failed to occur, that + * is, the update failed, so no need to anything in that case. + */ + if (insert_destrel && + resultRelInfo->ri_TrigDesc && + resultRelInfo->ri_TrigDesc->trig_update_after_row) + ExecCrossPartitionUpdateForeignKey(resultRelInfo, + tupleid, oldslot, + inserted_tuple, + mtstate, estate); + + return returning_slot; } /* @@ -1942,7 +2154,8 @@ lreplace:; (estate->es_processed)++; /* AFTER ROW UPDATE Triggers */ - ExecARUpdateTriggers(estate, resultRelInfo, tupleid, oldtuple, slot, + ExecARUpdateTriggers(estate, mtstate, resultRelInfo, tupleid, oldtuple, + slot, recheckIndexes, mtstate->operation == CMD_INSERT ? mtstate->mt_oc_transition_capture : @@ -2559,7 +2772,7 @@ ExecModifyTable(PlanState *pstate) ExecInitInsertProjection(node, resultRelInfo); slot = ExecGetInsertNewTuple(resultRelInfo, planSlot); slot = ExecInsert(node, resultRelInfo, slot, planSlot, - estate, node->canSetTag); + estate, node->canSetTag, NULL, NULL); break; case CMD_UPDATE: /* Initialize projection info if first time for this table */ diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 96269fc2ad..f04b2d87b6 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -1267,11 +1267,20 @@ RI_FKey_fk_upd_check_required(Trigger *trigger, Relation fk_rel, * not do anything; so we had better do the UPDATE check. (We could skip * this if we knew the INSERT trigger already fired, but there is no easy * way to know that.) + * + * Skip the check and just ask to fire the trigger if the FK relation is + * a partitioned table, because we can't inspect system columns of the + * tuple in that case. */ - xminDatum = slot_getsysattr(oldslot, MinTransactionIdAttributeNumber, &isnull); - Assert(!isnull); - xmin = DatumGetTransactionId(xminDatum); - if (TransactionIdIsCurrentTransactionId(xmin)) + if (fk_rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) + { + xminDatum = slot_getsysattr(oldslot, MinTransactionIdAttributeNumber, &isnull); + Assert(!isnull); + xmin = DatumGetTransactionId(xminDatum); + if (TransactionIdIsCurrentTransactionId(xmin)) + return true; + } + else return true; /* If all old and new key values are equal, no check is needed */ diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 8542705c5f..c10b850264 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -211,6 +211,7 @@ extern bool ExecBRDeleteTriggers(EState *estate, HeapTuple fdw_trigtuple, TupleTableSlot **epqslot); extern void ExecARDeleteTriggers(EState *estate, + ModifyTableState *mtstate, ResultRelInfo *relinfo, ItemPointer tupleid, HeapTuple fdw_trigtuple, @@ -230,6 +231,7 @@ extern bool ExecBRUpdateTriggers(EState *estate, HeapTuple fdw_trigtuple, TupleTableSlot *slot); extern void ExecARUpdateTriggers(EState *estate, + ModifyTableState *mtstate, ResultRelInfo *relinfo, ItemPointer tupleid, HeapTuple fdw_trigtuple, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 37cb4f3d59..4dd55ae792 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -524,6 +524,9 @@ typedef struct ResultRelInfo /* for use by copyfrom.c when performing multi-inserts */ struct CopyMultiInsertBuffer *ri_CopyMultiInsertBuffer; + + /* Used during cross-partition updates on partitioned tables. */ + List *ri_ancestorResultRels; } ResultRelInfo; /* ---------------- diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index bf794dce9d..6b5c2d8ac3 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -2485,7 +2485,7 @@ DELETE FROM pk WHERE a = 20; ERROR: update or delete on table "pk11" violates foreign key constraint "fk_a_fkey2" on table "fk" DETAIL: Key (a)=(20) is still referenced from table "fk". UPDATE pk SET a = 90 WHERE a = 30; -ERROR: update or delete on table "pk11" violates foreign key constraint "fk_a_fkey2" on table "fk" +ERROR: update or delete on table "pk" violates foreign key constraint "fk_a_fkey" on table "fk" DETAIL: Key (a)=(30) is still referenced from table "fk". SELECT tableoid::regclass, * FROM fk; tableoid | a @@ -2554,15 +2554,210 @@ CREATE SCHEMA fkpart10 CREATE TABLE tbl1(f1 int PRIMARY KEY) PARTITION BY RANGE(f1) CREATE TABLE tbl1_p1 PARTITION OF tbl1 FOR VALUES FROM (minvalue) TO (1) CREATE TABLE tbl1_p2 PARTITION OF tbl1 FOR VALUES FROM (1) TO (maxvalue) - CREATE TABLE tbl2(f1 int REFERENCES tbl1 DEFERRABLE INITIALLY DEFERRED); + CREATE TABLE tbl2(f1 int REFERENCES tbl1 DEFERRABLE INITIALLY DEFERRED) + CREATE TABLE tbl3(f1 int PRIMARY KEY) PARTITION BY RANGE(f1) + CREATE TABLE tbl3_p1 PARTITION OF tbl3 FOR VALUES FROM (minvalue) TO (1) + CREATE TABLE tbl3_p2 PARTITION OF tbl3 FOR VALUES FROM (1) TO (maxvalue) + CREATE TABLE tbl4(f1 int REFERENCES tbl3 DEFERRABLE INITIALLY DEFERRED); INSERT INTO fkpart10.tbl1 VALUES (0), (1); INSERT INTO fkpart10.tbl2 VALUES (0), (1); +INSERT INTO fkpart10.tbl3 VALUES (-2), (-1), (0); +INSERT INTO fkpart10.tbl4 VALUES (-2), (-1); BEGIN; DELETE FROM fkpart10.tbl1 WHERE f1 = 0; UPDATE fkpart10.tbl1 SET f1 = 2 WHERE f1 = 1; INSERT INTO fkpart10.tbl1 VALUES (0), (1); COMMIT; +-- test that cross-partition updates correctly enforces the foreign key +-- restriction (specifically testing INITIAILLY DEFERRED) +BEGIN; +UPDATE fkpart10.tbl1 SET f1 = 3 WHERE f1 = 0; +UPDATE fkpart10.tbl3 SET f1 = f1 * -1; +INSERT INTO fkpart10.tbl1 VALUES (4); +COMMIT; +ERROR: update or delete on table "tbl1" violates foreign key constraint "tbl2_f1_fkey" on table "tbl2" +DETAIL: Key (f1)=(0) is still referenced from table "tbl2". +BEGIN; +UPDATE fkpart10.tbl3 SET f1 = f1 * -1; +UPDATE fkpart10.tbl3 SET f1 = f1 + 3; +UPDATE fkpart10.tbl1 SET f1 = 3 WHERE f1 = 0; +INSERT INTO fkpart10.tbl1 VALUES (0); +COMMIT; +ERROR: update or delete on table "tbl3" violates foreign key constraint "tbl4_f1_fkey" on table "tbl4" +DETAIL: Key (f1)=(-2) is still referenced from table "tbl4". +BEGIN; +UPDATE fkpart10.tbl3 SET f1 = f1 * -1; +UPDATE fkpart10.tbl1 SET f1 = 3 WHERE f1 = 0; +INSERT INTO fkpart10.tbl1 VALUES (0); +INSERT INTO fkpart10.tbl3 VALUES (-2), (-1); +COMMIT; +-- test where the updated table now has both an IMMEDIATE and a DEFERRED +-- constraint pointing into it +CREATE TABLE fkpart10.tbl5(f1 int REFERENCES fkpart10.tbl3); +INSERT INTO fkpart10.tbl5 VALUES (-2), (-1); +BEGIN; +UPDATE fkpart10.tbl3 SET f1 = f1 * -3; +ERROR: update or delete on table "tbl3" violates foreign key constraint "tbl5_f1_fkey" on table "tbl5" +DETAIL: Key (f1)=(-2) is still referenced from table "tbl5". +COMMIT; +-- Now test where the row referenced from the table with an IMMEDIATE +-- constraint stays in place, while those referenced from the table with a +-- DEFERRED constraint don't. +DELETE FROM fkpart10.tbl5; +INSERT INTO fkpart10.tbl5 VALUES (0); +BEGIN; +UPDATE fkpart10.tbl3 SET f1 = f1 * -3; +COMMIT; +ERROR: update or delete on table "tbl3" violates foreign key constraint "tbl4_f1_fkey" on table "tbl4" +DETAIL: Key (f1)=(-2) is still referenced from table "tbl4". DROP SCHEMA fkpart10 CASCADE; -NOTICE: drop cascades to 2 other objects +NOTICE: drop cascades to 5 other objects DETAIL: drop cascades to table fkpart10.tbl1 drop cascades to table fkpart10.tbl2 +drop cascades to table fkpart10.tbl3 +drop cascades to table fkpart10.tbl4 +drop cascades to table fkpart10.tbl5 +-- verify foreign keys are enforced during cross-partition updates, +-- especially on the PK side +CREATE SCHEMA fkpart11 + CREATE TABLE pk (a INT PRIMARY KEY) PARTITION BY LIST (a) + CREATE TABLE fk ( + a INT, + CONSTRAINT fkey FOREIGN KEY (a) REFERENCES pk(a) ON UPDATE CASCADE ON DELETE CASCADE + ) + CREATE TABLE fk_parted ( + a INT PRIMARY KEY, + CONSTRAINT fkey FOREIGN KEY (a) REFERENCES pk(a) ON UPDATE CASCADE ON DELETE CASCADE + ) PARTITION BY LIST (a) + CREATE TABLE fk_another ( + a INT, + CONSTRAINT fkey FOREIGN KEY (a) REFERENCES fk_parted (a) ON UPDATE CASCADE ON DELETE CASCADE + ) + CREATE TABLE pk1 PARTITION OF pk FOR VALUES IN (1, 2) PARTITION BY LIST (a) + CREATE TABLE pk11 PARTITION OF pk1 FOR VALUES IN (1) + CREATE TABLE pk12 PARTITION OF pk1 FOR VALUES IN (2) + CREATE TABLE pk2 PARTITION OF pk FOR VALUES IN (3) + CREATE TABLE pk3 PARTITION OF pk FOR VALUES IN (4) + CREATE TABLE fk1 PARTITION OF fk_parted FOR VALUES IN (1, 2) + CREATE TABLE fk2 PARTITION OF fk_parted FOR VALUES IN (3) + CREATE TABLE fk3 PARTITION OF fk_parted FOR VALUES IN (4); +INSERT INTO fkpart11.pk VALUES (1), (3); +INSERT INTO fkpart11.fk VALUES (1), (3); +INSERT INTO fkpart11.fk_parted VALUES (1), (3); +INSERT INTO fkpart11.fk_another VALUES (1), (3); +-- moves 2 rows from one leaf partition to another, with both updates being +-- cascaded to fk and fk_parted. Updates of fk_parted, of which one is +-- cross-partition (3 -> 4), are further cascaded to fk_another. +UPDATE fkpart11.pk SET a = a + 1 RETURNING tableoid::pg_catalog.regclass, *; + tableoid | a +---------------+--- + fkpart11.pk12 | 2 + fkpart11.pk3 | 4 +(2 rows) + +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk; + tableoid | a +-------------+--- + fkpart11.fk | 2 + fkpart11.fk | 4 +(2 rows) + +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk_parted; + tableoid | a +--------------+--- + fkpart11.fk1 | 2 + fkpart11.fk3 | 4 +(2 rows) + +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk_another; + tableoid | a +---------------------+--- + fkpart11.fk_another | 2 + fkpart11.fk_another | 4 +(2 rows) + +-- let's try with the foreign key pointing at tables in the partition tree +-- that are not the same as the query's target table +-- 1. foreign key pointing into a non-root ancestor +-- +-- A cross-partition update on the root table will fail, because we currently +-- can't enforce the foreign keys pointing into a non-leaf partition +ALTER TABLE fkpart11.fk DROP CONSTRAINT fkey; +DELETE FROM fkpart11.fk WHERE a = 4; +ALTER TABLE fkpart11.fk ADD CONSTRAINT fkey FOREIGN KEY (a) REFERENCES fkpart11.pk1 (a) ON UPDATE CASCADE ON DELETE CASCADE; +UPDATE fkpart11.pk SET a = a - 1; +ERROR: cannot move tuple across partitions when non-root ancestor is directly referenced in a foreign key +DETAIL: A foreign key points to ancestor "pk1", but not the root ancestor "pk". +HINT: Consider defining the foreign key on "pk". +-- it's okay though if the non-leaf partition is updated directly +UPDATE fkpart11.pk1 SET a = a - 1; +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.pk; + tableoid | a +---------------+--- + fkpart11.pk11 | 1 + fkpart11.pk3 | 4 +(2 rows) + +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk; + tableoid | a +-------------+--- + fkpart11.fk | 1 +(1 row) + +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk_parted; + tableoid | a +--------------+--- + fkpart11.fk1 | 1 + fkpart11.fk3 | 4 +(2 rows) + +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk_another; + tableoid | a +---------------------+--- + fkpart11.fk_another | 4 + fkpart11.fk_another | 1 +(2 rows) + +-- 2. foreign key pointing into a single leaf partition +-- +-- A cross-partition update that deletes from the pointed-to leaf partition +-- is allowed to succeed +ALTER TABLE fkpart11.fk DROP CONSTRAINT fkey; +ALTER TABLE fkpart11.fk ADD CONSTRAINT fkey FOREIGN KEY (a) REFERENCES fkpart11.pk11 (a) ON UPDATE CASCADE ON DELETE CASCADE; +-- will delete (1) from p11 which is cascaded to fk +UPDATE fkpart11.pk SET a = a + 1 WHERE a = 1; +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk; + tableoid | a +----------+--- +(0 rows) + +DROP TABLE fkpart11.fk; +-- check that regular and deferrable AR triggers on the PK tables +-- still work as expected +CREATE FUNCTION fkpart11.print_row () RETURNS TRIGGER LANGUAGE plpgsql AS $$ + BEGIN + RAISE NOTICE 'TABLE: %, OP: %, OLD: %, NEW: %', TG_RELNAME, TG_OP, OLD, NEW; + RETURN NULL; + END; +$$; +CREATE TRIGGER trig_upd_pk AFTER UPDATE ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +CREATE TRIGGER trig_del_pk AFTER DELETE ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +CREATE TRIGGER trig_ins_pk AFTER INSERT ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +CREATE CONSTRAINT TRIGGER trig_upd_fk_parted AFTER UPDATE ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +CREATE CONSTRAINT TRIGGER trig_del_fk_parted AFTER DELETE ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +CREATE CONSTRAINT TRIGGER trig_ins_fk_parted AFTER INSERT ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +UPDATE fkpart11.pk SET a = 3 WHERE a = 4; +NOTICE: TABLE: pk3, OP: DELETE, OLD: (4), NEW: +NOTICE: TABLE: pk2, OP: INSERT, OLD: , NEW: (3) +NOTICE: TABLE: fk3, OP: DELETE, OLD: (4), NEW: +NOTICE: TABLE: fk2, OP: INSERT, OLD: , NEW: (3) +UPDATE fkpart11.pk SET a = 1 WHERE a = 2; +NOTICE: TABLE: pk12, OP: DELETE, OLD: (2), NEW: +NOTICE: TABLE: pk11, OP: INSERT, OLD: , NEW: (1) +NOTICE: TABLE: fk1, OP: UPDATE, OLD: (2), NEW: (1) +DROP SCHEMA fkpart11 CASCADE; +NOTICE: drop cascades to 4 other objects +DETAIL: drop cascades to table fkpart11.pk +drop cascades to table fkpart11.fk_parted +drop cascades to table fkpart11.fk_another +drop cascades to function fkpart11.print_row() diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index de417b62b6..88df1d791e 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1820,12 +1820,142 @@ CREATE SCHEMA fkpart10 CREATE TABLE tbl1(f1 int PRIMARY KEY) PARTITION BY RANGE(f1) CREATE TABLE tbl1_p1 PARTITION OF tbl1 FOR VALUES FROM (minvalue) TO (1) CREATE TABLE tbl1_p2 PARTITION OF tbl1 FOR VALUES FROM (1) TO (maxvalue) - CREATE TABLE tbl2(f1 int REFERENCES tbl1 DEFERRABLE INITIALLY DEFERRED); + CREATE TABLE tbl2(f1 int REFERENCES tbl1 DEFERRABLE INITIALLY DEFERRED) + CREATE TABLE tbl3(f1 int PRIMARY KEY) PARTITION BY RANGE(f1) + CREATE TABLE tbl3_p1 PARTITION OF tbl3 FOR VALUES FROM (minvalue) TO (1) + CREATE TABLE tbl3_p2 PARTITION OF tbl3 FOR VALUES FROM (1) TO (maxvalue) + CREATE TABLE tbl4(f1 int REFERENCES tbl3 DEFERRABLE INITIALLY DEFERRED); INSERT INTO fkpart10.tbl1 VALUES (0), (1); INSERT INTO fkpart10.tbl2 VALUES (0), (1); +INSERT INTO fkpart10.tbl3 VALUES (-2), (-1), (0); +INSERT INTO fkpart10.tbl4 VALUES (-2), (-1); BEGIN; DELETE FROM fkpart10.tbl1 WHERE f1 = 0; UPDATE fkpart10.tbl1 SET f1 = 2 WHERE f1 = 1; INSERT INTO fkpart10.tbl1 VALUES (0), (1); COMMIT; + +-- test that cross-partition updates correctly enforces the foreign key +-- restriction (specifically testing INITIAILLY DEFERRED) +BEGIN; +UPDATE fkpart10.tbl1 SET f1 = 3 WHERE f1 = 0; +UPDATE fkpart10.tbl3 SET f1 = f1 * -1; +INSERT INTO fkpart10.tbl1 VALUES (4); +COMMIT; + +BEGIN; +UPDATE fkpart10.tbl3 SET f1 = f1 * -1; +UPDATE fkpart10.tbl3 SET f1 = f1 + 3; +UPDATE fkpart10.tbl1 SET f1 = 3 WHERE f1 = 0; +INSERT INTO fkpart10.tbl1 VALUES (0); +COMMIT; + +BEGIN; +UPDATE fkpart10.tbl3 SET f1 = f1 * -1; +UPDATE fkpart10.tbl1 SET f1 = 3 WHERE f1 = 0; +INSERT INTO fkpart10.tbl1 VALUES (0); +INSERT INTO fkpart10.tbl3 VALUES (-2), (-1); +COMMIT; + +-- test where the updated table now has both an IMMEDIATE and a DEFERRED +-- constraint pointing into it +CREATE TABLE fkpart10.tbl5(f1 int REFERENCES fkpart10.tbl3); +INSERT INTO fkpart10.tbl5 VALUES (-2), (-1); +BEGIN; +UPDATE fkpart10.tbl3 SET f1 = f1 * -3; +COMMIT; + +-- Now test where the row referenced from the table with an IMMEDIATE +-- constraint stays in place, while those referenced from the table with a +-- DEFERRED constraint don't. +DELETE FROM fkpart10.tbl5; +INSERT INTO fkpart10.tbl5 VALUES (0); +BEGIN; +UPDATE fkpart10.tbl3 SET f1 = f1 * -3; +COMMIT; + DROP SCHEMA fkpart10 CASCADE; + +-- verify foreign keys are enforced during cross-partition updates, +-- especially on the PK side +CREATE SCHEMA fkpart11 + CREATE TABLE pk (a INT PRIMARY KEY) PARTITION BY LIST (a) + CREATE TABLE fk ( + a INT, + CONSTRAINT fkey FOREIGN KEY (a) REFERENCES pk(a) ON UPDATE CASCADE ON DELETE CASCADE + ) + CREATE TABLE fk_parted ( + a INT PRIMARY KEY, + CONSTRAINT fkey FOREIGN KEY (a) REFERENCES pk(a) ON UPDATE CASCADE ON DELETE CASCADE + ) PARTITION BY LIST (a) + CREATE TABLE fk_another ( + a INT, + CONSTRAINT fkey FOREIGN KEY (a) REFERENCES fk_parted (a) ON UPDATE CASCADE ON DELETE CASCADE + ) + CREATE TABLE pk1 PARTITION OF pk FOR VALUES IN (1, 2) PARTITION BY LIST (a) + CREATE TABLE pk11 PARTITION OF pk1 FOR VALUES IN (1) + CREATE TABLE pk12 PARTITION OF pk1 FOR VALUES IN (2) + CREATE TABLE pk2 PARTITION OF pk FOR VALUES IN (3) + CREATE TABLE pk3 PARTITION OF pk FOR VALUES IN (4) + CREATE TABLE fk1 PARTITION OF fk_parted FOR VALUES IN (1, 2) + CREATE TABLE fk2 PARTITION OF fk_parted FOR VALUES IN (3) + CREATE TABLE fk3 PARTITION OF fk_parted FOR VALUES IN (4); +INSERT INTO fkpart11.pk VALUES (1), (3); +INSERT INTO fkpart11.fk VALUES (1), (3); +INSERT INTO fkpart11.fk_parted VALUES (1), (3); +INSERT INTO fkpart11.fk_another VALUES (1), (3); +-- moves 2 rows from one leaf partition to another, with both updates being +-- cascaded to fk and fk_parted. Updates of fk_parted, of which one is +-- cross-partition (3 -> 4), are further cascaded to fk_another. +UPDATE fkpart11.pk SET a = a + 1 RETURNING tableoid::pg_catalog.regclass, *; +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk; +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk_parted; +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk_another; + +-- let's try with the foreign key pointing at tables in the partition tree +-- that are not the same as the query's target table + +-- 1. foreign key pointing into a non-root ancestor +-- +-- A cross-partition update on the root table will fail, because we currently +-- can't enforce the foreign keys pointing into a non-leaf partition +ALTER TABLE fkpart11.fk DROP CONSTRAINT fkey; +DELETE FROM fkpart11.fk WHERE a = 4; +ALTER TABLE fkpart11.fk ADD CONSTRAINT fkey FOREIGN KEY (a) REFERENCES fkpart11.pk1 (a) ON UPDATE CASCADE ON DELETE CASCADE; +UPDATE fkpart11.pk SET a = a - 1; +-- it's okay though if the non-leaf partition is updated directly +UPDATE fkpart11.pk1 SET a = a - 1; +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.pk; +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk; +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk_parted; +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk_another; + +-- 2. foreign key pointing into a single leaf partition +-- +-- A cross-partition update that deletes from the pointed-to leaf partition +-- is allowed to succeed +ALTER TABLE fkpart11.fk DROP CONSTRAINT fkey; +ALTER TABLE fkpart11.fk ADD CONSTRAINT fkey FOREIGN KEY (a) REFERENCES fkpart11.pk11 (a) ON UPDATE CASCADE ON DELETE CASCADE; +-- will delete (1) from p11 which is cascaded to fk +UPDATE fkpart11.pk SET a = a + 1 WHERE a = 1; +SELECT tableoid::pg_catalog.regclass, * FROM fkpart11.fk; +DROP TABLE fkpart11.fk; + +-- check that regular and deferrable AR triggers on the PK tables +-- still work as expected +CREATE FUNCTION fkpart11.print_row () RETURNS TRIGGER LANGUAGE plpgsql AS $$ + BEGIN + RAISE NOTICE 'TABLE: %, OP: %, OLD: %, NEW: %', TG_RELNAME, TG_OP, OLD, NEW; + RETURN NULL; + END; +$$; +CREATE TRIGGER trig_upd_pk AFTER UPDATE ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +CREATE TRIGGER trig_del_pk AFTER DELETE ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +CREATE TRIGGER trig_ins_pk AFTER INSERT ON fkpart11.pk FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +CREATE CONSTRAINT TRIGGER trig_upd_fk_parted AFTER UPDATE ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +CREATE CONSTRAINT TRIGGER trig_del_fk_parted AFTER DELETE ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +CREATE CONSTRAINT TRIGGER trig_ins_fk_parted AFTER INSERT ON fkpart11.fk_parted INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION fkpart11.print_row(); +UPDATE fkpart11.pk SET a = 3 WHERE a = 4; +UPDATE fkpart11.pk SET a = 1 WHERE a = 2; + +DROP SCHEMA fkpart11 CASCADE; -- 2.24.1