hi. V3 is attached. this should be more neat. Now it looks very similar to how statistics cope with column data type change.
For statistics, in ATPostAlterTypeParse we will call transformStatsStmt, we need to do the similar thing for the trigger WHEN clause. I introduced transformTriggerStmt and placed it in src/backend/commands/trigger.c, which should be fine, I think. It will be invoked from within CreateTriggerFiringOn. -- jian https://www.enterprisedb.com/
From dbbaa545db83c448b4e383df2e413a61915a8bee Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Thu, 22 Jan 2026 20:17:58 +0800 Subject: [PATCH v3 1/1] let SET DATA TYPE cope with trigger dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, we can not change column data type if any trigger depend on it. This is to removes that restriction. The general workflow is very similar to how STATISTICS are dropped and recreated. We collect all affected triggers, get their definitions, drop the existing triggers, and then recreate them. Any comments associated with the triggers are also dropped and recreated. Foreign key–related internal triggers are not directly dependent on the relation itself; instead, they depend directly on the constraint. Therefore, we don't need to worry about internal triggers here. Trigger containing whole-row references is handled in another thread, see https://postgr.es/m/CACJufxGA6KVQy7DbHGLVw9s9KKmpGyZt5ME6C7kEfjDpr2wZCw@mail.gmail.com discussion: https://postgr.es/m/CACJufxGkqYrmwMdvUOUPet0443oUTgF_dKCpw3TfJiutfuywAQ@mail.gmail.com commitfest: https://commitfest.postgresql.org/patch/6089 --- src/backend/catalog/index.c | 2 + src/backend/commands/tablecmds.c | 154 ++++++++++++++++-- src/backend/commands/trigger.c | 146 +++++++++++++++-- src/backend/parser/gram.y | 4 + src/backend/utils/adt/ruleutils.c | 10 ++ src/include/commands/trigger.h | 3 + src/include/nodes/parsenodes.h | 3 + src/include/utils/ruleutils.h | 1 + .../test_ddl_deparse/test_ddl_deparse.c | 3 + src/test/regress/expected/foreign_data.out | 21 +++ src/test/regress/expected/triggers.out | 110 +++++++++++++ src/test/regress/sql/foreign_data.sql | 15 ++ src/test/regress/sql/triggers.sql | 51 ++++++ 13 files changed, 496 insertions(+), 27 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 43de42ce39e..79e4e248603 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2037,6 +2037,8 @@ index_constraint_create(Relation heapRelation, trigger->deferrable = true; trigger->initdeferred = initdeferred; trigger->constrrel = NULL; + trigger->transformed = true; + trigger->trigcomment = NULL; (void) CreateTrigger(trigger, NULL, RelationGetRelid(heapRelation), InvalidOid, conOid, indexRelationId, InvalidOid, diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f976c0e5c7e..a892bd4d001 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -208,6 +208,8 @@ typedef struct AlteredTableInfo char *clusterOnIndex; /* index to use for CLUSTER */ List *changedStatisticsOids; /* OIDs of statistics to rebuild */ List *changedStatisticsDefs; /* string definitions of same */ + List *changedTriggerOids; /* OIDs of trigger to rebuild */ + List *changedTriggerDefs; /* string definitions of same */ } AlteredTableInfo; /* Struct describing one new constraint to check in Phase 3 scan */ @@ -546,6 +548,8 @@ static ObjectAddress ATExecAddIndex(AlteredTableInfo *tab, Relation rel, IndexStmt *stmt, bool is_rebuild, LOCKMODE lockmode); static ObjectAddress ATExecAddStatistics(AlteredTableInfo *tab, Relation rel, CreateStatsStmt *stmt, bool is_rebuild, LOCKMODE lockmode); +static ObjectAddress ATExecAddTrigger(AlteredTableInfo *tab, Relation rel, CreateTrigStmt *stmt, + bool is_rebuild, LOCKMODE lockmode); static ObjectAddress ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, Constraint *newConstraint, bool recurse, bool is_readd, @@ -651,6 +655,7 @@ static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableT static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab); static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab); static void RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab); +static void RememberTriggerForRebuilding(Oid trigoid, AlteredTableInfo *tab); static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode); static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, @@ -5465,6 +5470,10 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, address = ATExecAddStatistics(tab, rel, (CreateStatsStmt *) cmd->def, true, lockmode); break; + case AT_ReAddTrigger: /* ADD TRIGGER */ + address = ATExecAddTrigger(tab, rel, castNode(CreateTrigStmt, cmd->def), + true, lockmode); + break; case AT_AddConstraint: /* ADD CONSTRAINT */ /* Transform the command only during initial examination */ if (cur_pass == AT_PASS_ADD_CONSTR) @@ -6752,6 +6761,8 @@ alter_table_type_to_string(AlterTableType cmdtype) return "ALTER COLUMN ... DROP IDENTITY"; case AT_ReAddStatistics: return NULL; /* not real grammar */ + case AT_ReAddTrigger: + return NULL; /* not real grammar */ } return NULL; @@ -9725,6 +9736,30 @@ ATExecAddStatistics(AlteredTableInfo *tab, Relation rel, return address; } +/* + * ALTER TABLE ADD TRIGGER + * + * This is no such command in the grammar, but we use this internally to add + * AT_ReAddTrigger subcommands to rebuild trigger after a table + * column type change. + */ +static ObjectAddress +ATExecAddTrigger(AlteredTableInfo *tab, Relation rel, + CreateTrigStmt *stmt, bool is_rebuild, LOCKMODE lockmode) +{ + ObjectAddress address; + + Assert(IsA(stmt, CreateTrigStmt)); + + /* The CreateTrigStmt has already been through transformTriggerStmt */ + Assert(stmt->transformed); + + address = CreateTrigger(castNode(CreateTrigStmt, stmt), NULL, + InvalidOid, InvalidOid, InvalidOid, InvalidOid, + InvalidOid, InvalidOid, NULL, false, false); + return address; +} + /* * ALTER TABLE ADD CONSTRAINT USING INDEX * @@ -13864,6 +13899,8 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint, fk_trigger->deferrable = fkconstraint->deferrable; fk_trigger->initdeferred = fkconstraint->initdeferred; fk_trigger->constrrel = NULL; + fk_trigger->transformed = true; + fk_trigger->trigcomment = NULL; trigAddress = CreateTrigger(fk_trigger, NULL, myRelOid, refRelOid, constraintOid, indexOid, InvalidOid, @@ -13909,6 +13946,8 @@ createForeignKeyActionTriggers(Oid myRelOid, Oid refRelOid, Constraint *fkconstr fk_trigger->whenClause = NULL; fk_trigger->transitionRels = NIL; fk_trigger->constrrel = NULL; + fk_trigger->transformed = true; + fk_trigger->trigcomment = NULL; switch (fkconstraint->fk_del_action) { @@ -13969,6 +14008,8 @@ createForeignKeyActionTriggers(Oid myRelOid, Oid refRelOid, Constraint *fkconstr fk_trigger->whenClause = NULL; fk_trigger->transitionRels = NIL; fk_trigger->constrrel = NULL; + fk_trigger->transformed = true; + fk_trigger->trigcomment = NULL; switch (fkconstraint->fk_upd_action) { @@ -15180,21 +15221,13 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, case TriggerRelationId: /* - * A trigger can depend on a column because the column is - * specified as an update target, or because the column is - * used in the trigger's WHEN condition. The first case would - * not require any extra work, but the second case would - * require updating the WHEN expression, which has the same - * issues as above. Since we can't easily tell which case - * applies, we punt for both. FIXME someday. + * Internally-generated trigger for a constraint will have + * internal dependency of the constraint. It won't have direct + * dependency with the relation. So no need to worry about + * internal trigger here. */ if (subtype == AT_AlterColumnType) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot alter type of a column used in a trigger definition"), - errdetail("%s depends on column \"%s\"", - getObjectDescription(&foundObject, false), - colName))); + RememberTriggerForRebuilding(foundObject.objectId, tab); break; case PolicyRelationId: @@ -15455,6 +15488,33 @@ RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab) } } +/* + * Subroutine for ATExecAlterColumnType: remember that a trigger object + * needs to be rebuilt (which we might already know). + */ +static void +RememberTriggerForRebuilding(Oid trigoid, AlteredTableInfo *tab) +{ + /* + * This de-duplication check is critical for two independent reasons: we + * mustn't try to recreate the same trigger object twice, and if the + * trigger object depends on more than one column whose type is to be + * altered, we must capture its definition string before applying any of + * the type changes. ruleutils.c will get confused if we ask again later. + */ + if (!list_member_oid(tab->changedTriggerOids, trigoid)) + { + /* OK, capture the trigger object's existing definition string */ + char *defstring = pg_get_triggerobjdef_string(trigoid); + + tab->changedTriggerOids = lappend_oid(tab->changedTriggerOids, + trigoid); + tab->changedTriggerDefs = lappend(tab->changedTriggerDefs, + defstring); + } +} + + /* * Cleanup after we've finished all the ALTER TYPE or SET EXPRESSION * operations for a particular relation. We have to drop and recreate all the @@ -15599,6 +15659,47 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) add_exact_object_address(&obj, objects); } + /* add dependencies for new triggers */ + forboth(oid_item, tab->changedTriggerOids, + def_item, tab->changedTriggerDefs) + { + Oid refRelId = InvalidOid; + Oid oldId = lfirst_oid(oid_item); + List *relids = TriggerGetRelations(oldId); + + Assert(list_length(relids) == 1 || list_length(relids) == 2); + + /* + * As above, make sure we have lock on the trigger object's table if + * it's not the same table. However, we take ShareRowExclusiveLock + * here, aligning with the lock level used in CreateTriggerFiringOn. + * + * CAUTION: this should be done after all cases that grab + * AccessExclusiveLock, else we risk causing deadlock due to needing + * to promote our table lock. + */ + foreach_oid(relid, relids) + { + if (relid != tab->relid) + LockRelationOid(relid, ShareRowExclusiveLock); + } + + /* + * refRelId is the RI trigger opposite relation OID. It is passed to + * function ATPostAlterTypeParse, where it will assigned to + * CreateTrigStmt->constrrelOid. + */ + if (list_length(relids) == 2) + refRelId = lsecond_oid(relids); + + ATPostAlterTypeParse(oldId, linitial_oid(relids), refRelId, + (char *) lfirst(def_item), + wqueue, lockmode, tab->rewrite); + + ObjectAddressSet(obj, TriggerRelationId, oldId); + add_exact_object_address(&obj, objects); + } + /* * Queue up command to restore replica identity index marking */ @@ -15647,9 +15748,9 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) } /* - * Parse the previously-saved definition string for a constraint, index or - * statistics object against the newly-established column data type(s), and - * queue up the resulting command parsetrees for execution. + * Parse the previously-saved definition string for a constraint, index, + * statistics object or trigger against the newly-established column data + * type(s), and queue up the resulting command parsetrees for execution. * * This might fail if, for example, you have a WHERE clause that uses an * operator that's not available for the new column type. @@ -15700,6 +15801,11 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, transformStatsStmt(oldRelId, (CreateStatsStmt *) stmt, cmd)); + else if (IsA(stmt, CreateTrigStmt)) + querytree_list = lappend(querytree_list, + transformTriggerStmt(oldRelId, + (CreateTrigStmt *) stmt, + cmd)); else querytree_list = lappend(querytree_list, stmt); } @@ -15850,6 +15956,20 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, tab->subcmds[AT_PASS_MISC] = lappend(tab->subcmds[AT_PASS_MISC], newcmd); } + else if (IsA(stm, CreateTrigStmt)) + { + CreateTrigStmt *stmt = (CreateTrigStmt *) stm; + AlterTableCmd *newcmd; + + /* keep the trigger object's comment */ + stmt->trigcomment = GetComment(oldId, TriggerRelationId, 0); + + newcmd = makeNode(AlterTableCmd); + newcmd->subtype = AT_ReAddTrigger; + newcmd->def = (Node *) stmt; + tab->subcmds[AT_PASS_MISC] = + lappend(tab->subcmds[AT_PASS_MISC], newcmd); + } else elog(ERROR, "unexpected statement type: %d", (int) nodeTag(stm)); @@ -20919,6 +21039,8 @@ CloneRowTriggersToPartition(Relation parent, Relation partition) trigStmt->deferrable = trigForm->tgdeferrable; trigStmt->initdeferred = trigForm->tginitdeferred; trigStmt->constrrel = NULL; /* passed separately */ + trigStmt->transformed = true; + trigStmt->trigcomment = NULL; CreateTriggerFiringOn(trigStmt, NULL, RelationGetRelid(partition), trigForm->tgconstrrelid, InvalidOid, InvalidOid, diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index d30fda660eb..2a5c6d71283 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -30,6 +30,7 @@ #include "catalog/pg_proc.h" #include "catalog/pg_trigger.h" #include "catalog/pg_type.h" +#include "commands/comment.h" #include "commands/trigger.h" #include "executor/executor.h" #include "miscadmin.h" @@ -213,6 +214,9 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString, else rel = table_openrv(stmt->relation, ShareRowExclusiveLock); + /* Do parse analysis on the WHEN clause if not already done */ + stmt = transformTriggerStmt(RelationGetRelid(rel), stmt, NULL); + /* * Triggers must be on tables or views, and there are additional * relation-type-specific restrictions. @@ -555,10 +559,10 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString, } /* - * Parse the WHEN clause, if any and we weren't passed an already - * transformed one. + * We already parsed the WHEN clause in transformTriggerStmt, but in some + * cases, we disallowed references to OLD/NEW. * - * Note that as a side effect, we fill whenRtable when parsing. If we got + * Note that as a side effect, we fill whenRtable when checking. If we got * an already parsed clause, this does not occur, which is what we want -- * no point in adding redundant dependencies below. */ @@ -569,6 +573,12 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString, List *varList; ListCell *lc; + /* + * transformTriggerStmt has already performed parse analysis on a + * non-NULL whenClause, no need do it again. + */ + whenClause = stmt->whenClause; + /* Set up a pstate to parse with */ pstate = make_parsestate(NULL); pstate->p_sourcetext = queryString; @@ -589,14 +599,6 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString, false, false); addNSItemToQuery(pstate, nsitem, false, true, true); - /* Transform expression. Copy to be sure we don't modify original */ - whenClause = transformWhereClause(pstate, - copyObject(stmt->whenClause), - EXPR_KIND_TRIGGER_WHEN, - "WHEN"); - /* we have to fix its collations too */ - assign_expr_collations(pstate, whenClause); - /* * Check for disallowed references to OLD/NEW. * @@ -1204,6 +1206,11 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString, /* Keep lock on target rel until end of xact */ table_close(rel, NoLock); + /* Add any requested comment */ + if (stmt->trigcomment != NULL) + CreateComments(trigoid, TriggerRelationId, 0, + stmt->trigcomment); + return myself; } @@ -1413,6 +1420,54 @@ get_trigger_oid(Oid relid, const char *trigname, bool missing_ok) return oid; } + /* + * TriggerGetRelations + * + * Collect all relations this trigger depends on. The constraint trigger may + * reference another relation, we include it as well. + */ +List * +TriggerGetRelations(Oid trigId) +{ + HeapTuple ht_trig; + ScanKeyData skey[1]; + SysScanDesc tgscan; + List *result = NIL; + + /* + * find the pg_trigger tuple by the Oid of the trigger + */ + Relation tgrel = table_open(TriggerRelationId, + AccessShareLock); + + ScanKeyInit(&skey[0], + Anum_pg_trigger_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(trigId)); + tgscan = systable_beginscan(tgrel, TriggerOidIndexId, true, + NULL, 1, skey); + ht_trig = systable_getnext(tgscan); + + if (HeapTupleIsValid(ht_trig)) + { + Form_pg_trigger trigrec = (Form_pg_trigger) GETSTRUCT(ht_trig); + + Assert(trigrec->oid = trigId); + + result = lappend_oid(result, trigrec->tgrelid); + + if (OidIsValid(trigrec->tgconstrrelid)) + result = lappend_oid(result, trigrec->tgconstrrelid); + } + + /* Clean up */ + systable_endscan(tgscan); + + table_close(tgrel, AccessShareLock); + + return result; +} + /* * Perform permissions and integrity checks before acquiring a relation lock. */ @@ -6715,3 +6770,72 @@ check_modified_virtual_generated(TupleDesc tupdesc, HeapTuple tuple) return tuple; } + +/* + * transformTriggerStmt - parse analysis for CREATE TRIGGER + * + * To avoid race conditions, it's important that this function relies only on + * the passed-in relid (and not on stmt->relation) to determine the target + * relation. + */ +CreateTrigStmt * +transformTriggerStmt(Oid relid, CreateTrigStmt *stmt, const char *queryString) +{ + ParseState *pstate; + ParseNamespaceItem *nsitem; + Relation rel; + + /* + * Nothing to do if statement already transformed. transformed is set to + * true if CreateTrigStmt has no whenClause. + */ + if (stmt->transformed) + return stmt; + else if (!stmt->whenClause) + { + stmt->transformed = true; + + return stmt; + } + + rel = relation_open(relid, NoLock); + + /* Set up a pstate to parse with */ + pstate = make_parsestate(NULL); + pstate->p_sourcetext = queryString; + + /* + * Set up nsitems for OLD and NEW references. + * + * 'OLD' must always have varno equal to 1 and 'NEW' equal to 2. + */ + nsitem = addRangeTableEntryForRelation(pstate, rel, + AccessShareLock, + makeAlias("old", NIL), + false, false); + addNSItemToQuery(pstate, nsitem, false, true, true); + + nsitem = addRangeTableEntryForRelation(pstate, rel, + AccessShareLock, + makeAlias("new", NIL), + false, false); + addNSItemToQuery(pstate, nsitem, false, true, true); + + /* Transform expression. Copy to be sure we don't modify original */ + stmt->whenClause = transformWhereClause(pstate, + copyObject(stmt->whenClause), + EXPR_KIND_TRIGGER_WHEN, + "WHEN"); + /* we have to fix its collations too */ + assign_expr_collations(pstate, stmt->whenClause); + + free_parsestate(pstate); + + /* Close relation */ + table_close(rel, NoLock); + + /* Mark statement as successfully transformed */ + stmt->transformed = true; + + return stmt; +} diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 713ee5c10a2..3cac4af1f09 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -6150,6 +6150,8 @@ CreateTrigStmt: n->deferrable = false; n->initdeferred = false; n->constrrel = NULL; + n->transformed = false; + n->trigcomment = NULL; $$ = (Node *) n; } | CREATE opt_or_replace CONSTRAINT TRIGGER name AFTER TriggerEvents ON @@ -6200,6 +6202,8 @@ CreateTrigStmt: &n->deferrable, &n->initdeferred, &dummy, NULL, NULL, yyscanner); n->constrrel = $10; + n->transformed = false; + n->trigcomment = NULL; $$ = (Node *) n; } ; diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 033b625f3fc..858a8845890 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -881,6 +881,16 @@ pg_get_triggerdef(PG_FUNCTION_ARGS) PG_RETURN_TEXT_P(string_to_text(res)); } +/* + * Internal version for use by ALTER TABLE. + * Returns a palloc'd C string with pretty-printing. + */ +char * +pg_get_triggerobjdef_string(Oid trigid) +{ + return pg_get_triggerdef_worker(trigid, true); +} + Datum pg_get_triggerdef_ext(PG_FUNCTION_ARGS) { diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index b60317c7a75..d9d703a8a7f 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -160,6 +160,8 @@ extern ObjectAddress CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *que Oid indexOid, Oid funcoid, Oid parentTriggerOid, Node *whenClause, bool isInternal, bool in_partition, char trigger_fires_when); +extern CreateTrigStmt *transformTriggerStmt(Oid relid, CreateTrigStmt *stmt, + const char *queryString); extern void TriggerSetParentTrigger(Relation trigRel, Oid childTrigId, @@ -167,6 +169,7 @@ extern void TriggerSetParentTrigger(Relation trigRel, Oid childTableId); extern void RemoveTriggerById(Oid trigOid); extern Oid get_trigger_oid(Oid relid, const char *trigname, bool missing_ok); +extern List *TriggerGetRelations(Oid trigId); extern ObjectAddress renametrig(RenameStmt *stmt); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 646d6ced763..b194dbd116f 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2509,6 +2509,7 @@ typedef enum AlterTableType AT_SetIdentity, /* SET identity column options */ AT_DropIdentity, /* DROP IDENTITY */ AT_ReAddStatistics, /* internal to commands/tablecmds.c */ + AT_ReAddTrigger, /* internal to commands/tablecmds.c */ } AlterTableType; typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */ @@ -3154,6 +3155,8 @@ typedef struct CreateTrigStmt bool deferrable; /* [NOT] DEFERRABLE */ bool initdeferred; /* INITIALLY {DEFERRED|IMMEDIATE} */ RangeVar *constrrel; /* opposite relation, if RI trigger */ + bool transformed; + char *trigcomment; /* comment to apply to trigger, or NULL */ } CreateTrigStmt; /* ---------------------- diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h index 908b2708ed4..cde806a00a1 100644 --- a/src/include/utils/ruleutils.h +++ b/src/include/utils/ruleutils.h @@ -53,5 +53,6 @@ extern char *generate_opclass_name(Oid opclass); extern char *get_range_partbound_string(List *bound_datums); extern char *pg_get_statisticsobjdef_string(Oid statextid); +extern char *pg_get_triggerobjdef_string(Oid trigid); #endif /* RULEUTILS_H */ diff --git a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c index 380b3e754b7..8f06ad5c0dd 100644 --- a/src/test/modules/test_ddl_deparse/test_ddl_deparse.c +++ b/src/test/modules/test_ddl_deparse/test_ddl_deparse.c @@ -314,6 +314,9 @@ get_altertable_subcmdinfo(PG_FUNCTION_ARGS) case AT_ReAddStatistics: strtype = "(re) ADD STATS"; break; + case AT_ReAddTrigger: + strtype = "(re) ADD TRIGGER"; + break; } if (subcmd->recurse) diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index cce49e509ab..cec5a86affc 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -1394,6 +1394,27 @@ ALTER FOREIGN TABLE foreign_schema.foreign_table_1 DISABLE TRIGGER trigtest_before_stmt; ALTER FOREIGN TABLE foreign_schema.foreign_table_1 ENABLE TRIGGER trigtest_before_stmt; +CREATE TRIGGER trigtest_before_rowwhen BEFORE UPDATE +ON foreign_schema.foreign_table_1 +FOR EACH ROW +WHEN (NEW.c7 = 1 OR OLD.c7 = 1) +EXECUTE PROCEDURE dummy_trigger(); +ALTER FOREIGN TABLE foreign_schema.foreign_table_1 ALTER COLUMN c7 SET DATA TYPE text; --error +ERROR: operator does not exist: text = integer +DETAIL: No operator of that name accepts the given argument types. +HINT: You might need to add explicit type casts. +ALTER FOREIGN TABLE foreign_schema.foreign_table_1 ALTER COLUMN c7 SET DATA TYPE numeric; +SELECT pg_get_triggerdef(oid, true) +FROM pg_trigger +WHERE tgrelid = 'foreign_schema.foreign_table_1'::regclass +AND tgname = 'trigtest_before_rowwhen'; + pg_get_triggerdef +---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + CREATE TRIGGER trigtest_before_rowwhen BEFORE UPDATE ON foreign_schema.foreign_table_1 FOR EACH ROW WHEN (new.c7 = 1::numeric OR old.c7 = 1::numeric) EXECUTE FUNCTION dummy_trigger() +(1 row) + +ALTER FOREIGN TABLE foreign_schema.foreign_table_1 ALTER COLUMN c7 SET DATA TYPE int; +DROP TRIGGER trigtest_before_rowwhen ON foreign_schema.foreign_table_1; DROP TRIGGER trigtest_before_stmt ON foreign_schema.foreign_table_1; DROP TRIGGER trigtest_before_row ON foreign_schema.foreign_table_1; DROP TRIGGER trigtest_after_stmt ON foreign_schema.foreign_table_1; diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 1eb8fba0953..2ee727a121b 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -261,6 +261,55 @@ SELECT trigger_name, event_manipulation, event_object_schema, event_object_table modified_any | UPDATE | public | main_table | 2 | (old.* IS DISTINCT FROM new.*) | ROW | BEFORE | | (10 rows) +PREPARE get_trigger_info(text[]) AS +SELECT event_object_table as trigger_table, event_object_schema as schema, trigger_name, event_manipulation as command_type, + action_order, action_condition, action_orientation, action_timing +FROM information_schema.triggers +WHERE event_object_table = ANY ($1) +ORDER BY trigger_name COLLATE "C", 2; +EXECUTE get_trigger_info('{main_table}'); + trigger_table | schema | trigger_name | command_type | action_order | action_condition | action_orientation | action_timing +---------------+--------+----------------------+--------------+--------------+--------------------------------+--------------------+--------------- + main_table | public | after_ins_stmt_trig | INSERT | 1 | | STATEMENT | AFTER + main_table | public | after_upd_row_trig | UPDATE | 1 | | ROW | AFTER + main_table | public | after_upd_stmt_trig | UPDATE | 1 | | STATEMENT | AFTER + main_table | public | before_ins_stmt_trig | INSERT | 1 | | STATEMENT | BEFORE + main_table | public | delete_a | DELETE | 1 | (old.a = 123) | ROW | AFTER + main_table | public | delete_when | DELETE | 1 | true | STATEMENT | AFTER + main_table | public | insert_a | INSERT | 1 | (new.a = 123) | ROW | AFTER + main_table | public | insert_when | INSERT | 2 | true | STATEMENT | BEFORE + main_table | public | modified_a | UPDATE | 1 | (old.a <> new.a) | ROW | BEFORE + main_table | public | modified_any | UPDATE | 2 | (old.* IS DISTINCT FROM new.*) | ROW | BEFORE +(10 rows) + +COMMENT ON TRIGGER modified_a ON main_table IS 'modified_a trigger'; +ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE text; --error +ERROR: operator does not exist: text = integer +DETAIL: No operator of that name accepts the given argument types. +HINT: You might need to add explicit type casts. +ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE numeric; --ok +EXECUTE get_trigger_info('{main_table}'); + trigger_table | schema | trigger_name | command_type | action_order | action_condition | action_orientation | action_timing +---------------+--------+----------------------+--------------+--------------+--------------------------------+--------------------+--------------- + main_table | public | after_ins_stmt_trig | INSERT | 1 | | STATEMENT | AFTER + main_table | public | after_upd_row_trig | UPDATE | 1 | | ROW | AFTER + main_table | public | after_upd_stmt_trig | UPDATE | 1 | | STATEMENT | AFTER + main_table | public | before_ins_stmt_trig | INSERT | 1 | | STATEMENT | BEFORE + main_table | public | delete_a | DELETE | 1 | (old.a = (123)::numeric) | ROW | AFTER + main_table | public | delete_when | DELETE | 1 | true | STATEMENT | AFTER + main_table | public | insert_a | INSERT | 1 | (new.a = (123)::numeric) | ROW | AFTER + main_table | public | insert_when | INSERT | 2 | true | STATEMENT | BEFORE + main_table | public | modified_a | UPDATE | 1 | (old.a <> new.a) | ROW | BEFORE + main_table | public | modified_any | UPDATE | 2 | (old.* IS DISTINCT FROM new.*) | ROW | BEFORE +(10 rows) + +\dd modified_a + Object descriptions + Schema | Name | Object | Description +--------+------------+---------+-------------------- + public | modified_a | trigger | modified_a trigger +(1 row) + INSERT INTO main_table (a) VALUES (123), (456); NOTICE: trigger_func(before_ins_stmt) called: action = INSERT, when = BEFORE, level = STATEMENT NOTICE: trigger_func(insert_when) called: action = INSERT, when = BEFORE, level = STATEMENT @@ -298,6 +347,13 @@ SELECT * FROM main_table ORDER BY a, b; | (8 rows) +SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'insert_a'; + pg_get_triggerdef +--------------------------------------------------------------------------------------------------------------------------------------- + CREATE TRIGGER insert_a AFTER INSERT ON main_table FOR EACH ROW WHEN (new.a = 123::numeric) EXECUTE FUNCTION trigger_func('insert_a') +(1 row) + +ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE integer; SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_a'; pg_get_triggerdef ------------------------------------------------------------------------------------------------------------------------------------------- @@ -336,6 +392,26 @@ DROP TRIGGER insert_a ON main_table; DROP TRIGGER delete_a ON main_table; DROP TRIGGER insert_when ON main_table; DROP TRIGGER delete_when ON main_table; +CREATE TABLE main_table1 (a text); +CREATE FUNCTION test_before_ins_trig () RETURNS TRIGGER LANGUAGE plpgsql +AS $$ +BEGIN + NEW.a = 'a'; + RETURN new; +END; +$$; +CREATE TRIGGER before_trig +BEFORE INSERT OR UPDATE ON main_table1 +FOR EACH ROW EXECUTE PROCEDURE test_before_ins_trig (); +ALTER TABLE main_table1 ALTER COLUMN a SET data TYPE int8 USING a::bigint; +INSERT INTO main_table1 VALUES (1); --error +ERROR: invalid input syntax for type bigint: "a" +LINE 1: NEW.a = 'a' + ^ +QUERY: NEW.a = 'a' +CONTEXT: PL/pgSQL function test_before_ins_trig() line 3 at assignment +DROP TABLE main_table1; +DROP FUNCTION test_before_ins_trig; -- Test WHEN condition accessing system columns. create table table_with_oids(a int); insert into table_with_oids values (1); @@ -343,6 +419,8 @@ create trigger oid_unchanged_trig after update on table_with_oids for each row when (new.tableoid = old.tableoid AND new.tableoid <> 0) execute procedure trigger_func('after_upd_oid_unchanged'); +alter table table_with_oids alter column a set data type text; --ok +alter table table_with_oids alter column a set data type int using a::integer; update table_with_oids set a = a + 1; NOTICE: trigger_func(after_upd_oid_unchanged) called: action = UPDATE, when = AFTER, level = ROW drop table table_with_oids; @@ -2165,6 +2243,21 @@ insert into parted_irreg_ancestor values ('aasvogel', 3); NOTICE: aasvogel <- woof! NOTICE: trigger parted_trig on parted1_irreg AFTER INSERT for ROW: (a,b)=(3,aasvogel) NOTICE: trigger parted_trig_odd on parted1_irreg AFTER INSERT for ROW: (a,b)=(3,aasvogel) +-- While the partitioned table has no triggers, individual partitions do. +-- Rebuild the partition's trigger may fail if the existing WHEN clause cannot +-- cope with the new data type. +alter table parted_irreg_ancestor alter column a set data type text; --error +ERROR: operator does not exist: text % integer +DETAIL: No operator of that name accepts the given argument types. +HINT: You might need to add explicit type casts. +alter table parted_irreg_ancestor alter column a set data type numeric; +execute get_trigger_info('{parted_irreg_ancestor, parted1_irreg}'); + trigger_table | schema | trigger_name | command_type | action_order | action_condition | action_orientation | action_timing +---------------+--------+-----------------+--------------+--------------+-----------------------------------------------------------+--------------------+--------------- + parted1_irreg | public | parted_trig | INSERT | 1 | | ROW | AFTER + parted1_irreg | public | parted_trig_odd | INSERT | 2 | (bark(new.b) AND ((new.a % (2)::numeric) = (1)::numeric)) | ROW | AFTER +(2 rows) + drop table parted_irreg_ancestor; -- Before triggers and partitions create table parted (a int, b int, c text) partition by list (a); @@ -2318,6 +2411,23 @@ create constraint trigger parted_trig_two after insert on parted_constr deferrable initially deferred enforced for each row when (bark(new.b) AND new.a % 2 = 1) execute procedure trigger_notice_ab(); +alter table parted_constr_ancestor alter column a set data type text; --error +ERROR: operator does not exist: text % integer +DETAIL: No operator of that name accepts the given argument types. +HINT: You might need to add explicit type casts. +alter table parted_constr_ancestor alter column a set data type numeric; +execute get_trigger_info('{parted_constr_ancestor, parted_constr, parted_constr_ancestor, parted1_constr}'); + trigger_table | schema | trigger_name | command_type | action_order | action_condition | action_orientation | action_timing +------------------------+--------+-----------------+--------------+--------------+-----------------------------------------------------------+--------------------+--------------- + parted1_constr | public | parted_trig | INSERT | 1 | | ROW | AFTER + parted_constr | public | parted_trig | INSERT | 1 | | ROW | AFTER + parted_constr_ancestor | public | parted_trig | INSERT | 1 | | ROW | AFTER + parted1_constr | public | parted_trig_two | INSERT | 2 | (bark(new.b) AND ((new.a % (2)::numeric) = (1)::numeric)) | ROW | AFTER + parted_constr | public | parted_trig_two | INSERT | 2 | (bark(new.b) AND ((new.a % (2)::numeric) = (1)::numeric)) | ROW | AFTER +(5 rows) + +alter table parted_constr_ancestor alter column a set data type int; +deallocate get_trigger_info; -- The immediate constraint is fired immediately; the WHEN clause of the -- deferred constraint is also called immediately. The deferred constraint -- is fired at commit time. diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql index aa147b14a90..156226e48e7 100644 --- a/src/test/regress/sql/foreign_data.sql +++ b/src/test/regress/sql/foreign_data.sql @@ -625,6 +625,21 @@ ALTER FOREIGN TABLE foreign_schema.foreign_table_1 ALTER FOREIGN TABLE foreign_schema.foreign_table_1 ENABLE TRIGGER trigtest_before_stmt; +CREATE TRIGGER trigtest_before_rowwhen BEFORE UPDATE +ON foreign_schema.foreign_table_1 +FOR EACH ROW +WHEN (NEW.c7 = 1 OR OLD.c7 = 1) +EXECUTE PROCEDURE dummy_trigger(); +ALTER FOREIGN TABLE foreign_schema.foreign_table_1 ALTER COLUMN c7 SET DATA TYPE text; --error +ALTER FOREIGN TABLE foreign_schema.foreign_table_1 ALTER COLUMN c7 SET DATA TYPE numeric; + +SELECT pg_get_triggerdef(oid, true) +FROM pg_trigger +WHERE tgrelid = 'foreign_schema.foreign_table_1'::regclass +AND tgname = 'trigtest_before_rowwhen'; +ALTER FOREIGN TABLE foreign_schema.foreign_table_1 ALTER COLUMN c7 SET DATA TYPE int; + +DROP TRIGGER trigtest_before_rowwhen ON foreign_schema.foreign_table_1; DROP TRIGGER trigtest_before_stmt ON foreign_schema.foreign_table_1; DROP TRIGGER trigtest_before_row ON foreign_schema.foreign_table_1; DROP TRIGGER trigtest_after_stmt ON foreign_schema.foreign_table_1; diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 5f7f75d7ba5..c18b2bcf8bd 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -180,6 +180,22 @@ SELECT trigger_name, event_manipulation, event_object_schema, event_object_table FROM information_schema.triggers WHERE event_object_table IN ('main_table') ORDER BY trigger_name COLLATE "C", 2; + +PREPARE get_trigger_info(text[]) AS +SELECT event_object_table as trigger_table, event_object_schema as schema, trigger_name, event_manipulation as command_type, + action_order, action_condition, action_orientation, action_timing +FROM information_schema.triggers +WHERE event_object_table = ANY ($1) +ORDER BY trigger_name COLLATE "C", 2; + +EXECUTE get_trigger_info('{main_table}'); +COMMENT ON TRIGGER modified_a ON main_table IS 'modified_a trigger'; + +ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE text; --error +ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE numeric; --ok +EXECUTE get_trigger_info('{main_table}'); +\dd modified_a + INSERT INTO main_table (a) VALUES (123), (456); COPY main_table FROM stdin; 123 999 @@ -188,6 +204,8 @@ COPY main_table FROM stdin; DELETE FROM main_table WHERE a IN (123, 456); UPDATE main_table SET a = 50, b = 60; SELECT * FROM main_table ORDER BY a, b; +SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'insert_a'; +ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE integer; SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_a'; SELECT pg_get_triggerdef(oid, false) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_a'; SELECT pg_get_triggerdef(oid, true) FROM pg_trigger WHERE tgrelid = 'main_table'::regclass AND tgname = 'modified_any'; @@ -204,6 +222,25 @@ DROP TRIGGER delete_a ON main_table; DROP TRIGGER insert_when ON main_table; DROP TRIGGER delete_when ON main_table; +CREATE TABLE main_table1 (a text); +CREATE FUNCTION test_before_ins_trig () RETURNS TRIGGER LANGUAGE plpgsql +AS $$ +BEGIN + NEW.a = 'a'; + RETURN new; +END; +$$; + +CREATE TRIGGER before_trig +BEFORE INSERT OR UPDATE ON main_table1 +FOR EACH ROW EXECUTE PROCEDURE test_before_ins_trig (); + +ALTER TABLE main_table1 ALTER COLUMN a SET data TYPE int8 USING a::bigint; +INSERT INTO main_table1 VALUES (1); --error + +DROP TABLE main_table1; +DROP FUNCTION test_before_ins_trig; + -- Test WHEN condition accessing system columns. create table table_with_oids(a int); insert into table_with_oids values (1); @@ -211,6 +248,8 @@ create trigger oid_unchanged_trig after update on table_with_oids for each row when (new.tableoid = old.tableoid AND new.tableoid <> 0) execute procedure trigger_func('after_upd_oid_unchanged'); +alter table table_with_oids alter column a set data type text; --ok +alter table table_with_oids alter column a set data type int using a::integer; update table_with_oids set a = a + 1; drop table table_with_oids; @@ -1495,6 +1534,12 @@ create trigger parted_trig_odd after insert on parted_irreg for each row insert into parted_irreg values (1, 'aardvark'), (2, 'aanimals'); insert into parted1_irreg values ('aardwolf', 2); insert into parted_irreg_ancestor values ('aasvogel', 3); +-- While the partitioned table has no triggers, individual partitions do. +-- Rebuild the partition's trigger may fail if the existing WHEN clause cannot +-- cope with the new data type. +alter table parted_irreg_ancestor alter column a set data type text; --error +alter table parted_irreg_ancestor alter column a set data type numeric; +execute get_trigger_info('{parted_irreg_ancestor, parted1_irreg}'); drop table parted_irreg_ancestor; -- Before triggers and partitions @@ -1608,6 +1653,12 @@ create constraint trigger parted_trig_two after insert on parted_constr for each row when (bark(new.b) AND new.a % 2 = 1) execute procedure trigger_notice_ab(); +alter table parted_constr_ancestor alter column a set data type text; --error +alter table parted_constr_ancestor alter column a set data type numeric; +execute get_trigger_info('{parted_constr_ancestor, parted_constr, parted_constr_ancestor, parted1_constr}'); +alter table parted_constr_ancestor alter column a set data type int; +deallocate get_trigger_info; + -- The immediate constraint is fired immediately; the WHEN clause of the -- deferred constraint is also called immediately. The deferred constraint -- is fired at commit time. -- 2.34.1
