From 19b053469519cb743469e4cb0670515fc936684a Mon Sep 17 00:00:00 2001 From: Amul Sul Date: Wed, 22 Nov 2023 18:23:56 +0530 Subject: [PATCH v7 2/3] Code refactor: separate function to find all dependent object on column Move code from ATExecAlterColumnType() that finds the all the object that depends on the column to a separate function. Also, renamed ATPostAlterTypeCleanup() and ATPostAlterTypeParse() function for the general use. --- src/backend/commands/tablecmds.c | 486 ++++++++++++++++--------------- 1 file changed, 254 insertions(+), 232 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index ce386199ac3..1beea526d8e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -561,14 +561,16 @@ static void ATPrepAlterColumnType(List **wqueue, static bool ATColumnChangeRequiresRewrite(Node *expr, AttrNumber varattno); static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, AlterTableCmd *cmd, LOCKMODE lockmode); +static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, + Relation rel, AttrNumber attnum); static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab); static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab); static void RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab); -static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, - LOCKMODE lockmode); -static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, - char *cmd, List **wqueue, LOCKMODE lockmode, - bool rewrite); +static void ATPostAlterColumnCleanup(List **wqueue, AlteredTableInfo *tab, + LOCKMODE lockmode); +static void ATPostAlterColumnParse(Oid oldId, Oid oldRelId, Oid refRelId, + char *cmd, List **wqueue, LOCKMODE lockmode, + bool rewrite); static void RebuildConstraintComment(AlteredTableInfo *tab, int pass, Oid objid, Relation rel, List *domname, const char *conname); @@ -5157,7 +5159,7 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, * multiple columns of a table are altered). */ if (pass == AT_PASS_ALTER_TYPE) - ATPostAlterTypeCleanup(wqueue, tab, lockmode); + ATPostAlterColumnCleanup(wqueue, tab, lockmode); if (tab->rel) { @@ -13292,222 +13294,18 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, /* * Find everything that depends on the column (constraints, indexes, etc), - * and record enough information to let us recreate the objects. - * - * The actual recreation does not happen here, but only after we have - * performed all the individual ALTER TYPE operations. We have to save - * the info before executing ALTER TYPE, though, else the deparser will - * get confused. + * and record enough information to let us recreate the objects after ALTER + * TYPE operations. */ - depRel = table_open(DependRelationId, RowExclusiveLock); - - ScanKeyInit(&key[0], - Anum_pg_depend_refclassid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationRelationId)); - ScanKeyInit(&key[1], - Anum_pg_depend_refobjid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationGetRelid(rel))); - ScanKeyInit(&key[2], - Anum_pg_depend_refobjsubid, - BTEqualStrategyNumber, F_INT4EQ, - Int32GetDatum((int32) attnum)); - - scan = systable_beginscan(depRel, DependReferenceIndexId, true, - NULL, 3, key); - - while (HeapTupleIsValid(depTup = systable_getnext(scan))) - { - Form_pg_depend foundDep = (Form_pg_depend) GETSTRUCT(depTup); - ObjectAddress foundObject; - - foundObject.classId = foundDep->classid; - foundObject.objectId = foundDep->objid; - foundObject.objectSubId = foundDep->objsubid; - - switch (getObjectClass(&foundObject)) - { - case OCLASS_CLASS: - { - char relKind = get_rel_relkind(foundObject.objectId); - - if (relKind == RELKIND_INDEX || - relKind == RELKIND_PARTITIONED_INDEX) - { - Assert(foundObject.objectSubId == 0); - RememberIndexForRebuilding(foundObject.objectId, tab); - } - else if (relKind == RELKIND_SEQUENCE) - { - /* - * This must be a SERIAL column's sequence. We need - * not do anything to it. - */ - Assert(foundObject.objectSubId == 0); - } - else - { - /* Not expecting any other direct dependencies... */ - elog(ERROR, "unexpected object depending on column: %s", - getObjectDescription(&foundObject, false)); - } - break; - } - - case OCLASS_CONSTRAINT: - Assert(foundObject.objectSubId == 0); - RememberConstraintForRebuilding(foundObject.objectId, tab); - break; - - case OCLASS_REWRITE: - /* XXX someday see if we can cope with revising views */ - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot alter type of a column used by a view or rule"), - errdetail("%s depends on column \"%s\"", - getObjectDescription(&foundObject, false), - colName))); - break; - - case OCLASS_TRIGGER: - - /* - * 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 will take a - * significant amount of new code. Since we can't easily tell - * which case applies, we punt for both. FIXME someday. - */ - 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))); - break; - - case OCLASS_POLICY: - - /* - * A policy can depend on a column because the column is - * specified in the policy's USING or WITH CHECK qual - * expressions. It might be possible to rewrite and recheck - * the policy expression, but punt for now. It's certainly - * easy enough to remove and recreate the policy; still, FIXME - * someday. - */ - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot alter type of a column used in a policy definition"), - errdetail("%s depends on column \"%s\"", - getObjectDescription(&foundObject, false), - colName))); - break; - - case OCLASS_DEFAULT: - { - ObjectAddress col = GetAttrDefaultColumnAddress(foundObject.objectId); - - if (col.objectId == RelationGetRelid(rel) && - col.objectSubId == attnum) - { - /* - * Ignore the column's own default expression, which - * we will deal with below. - */ - Assert(defaultexpr); - } - else - { - /* - * This must be a reference from the expression of a - * generated column elsewhere in the same table. - * Changing the type of a column that is used by a - * generated column is not allowed by SQL standard, so - * just punt for now. It might be doable with some - * thinking and effort. - */ - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot alter type of a column used by a generated column"), - errdetail("Column \"%s\" is used by generated column \"%s\".", - colName, - get_attname(col.objectId, - col.objectSubId, - false)))); - } - break; - } - - case OCLASS_STATISTIC_EXT: - - /* - * Give the extended-stats machinery a chance to fix anything - * that this column type change would break. - */ - RememberStatisticsForRebuilding(foundObject.objectId, tab); - break; - - case OCLASS_PROC: - case OCLASS_TYPE: - case OCLASS_CAST: - case OCLASS_COLLATION: - case OCLASS_CONVERSION: - case OCLASS_LANGUAGE: - case OCLASS_LARGEOBJECT: - case OCLASS_OPERATOR: - case OCLASS_OPCLASS: - case OCLASS_OPFAMILY: - case OCLASS_AM: - case OCLASS_AMOP: - case OCLASS_AMPROC: - case OCLASS_SCHEMA: - case OCLASS_TSPARSER: - case OCLASS_TSDICT: - case OCLASS_TSTEMPLATE: - case OCLASS_TSCONFIG: - case OCLASS_ROLE: - case OCLASS_ROLE_MEMBERSHIP: - case OCLASS_DATABASE: - case OCLASS_TBLSPACE: - case OCLASS_FDW: - case OCLASS_FOREIGN_SERVER: - case OCLASS_USER_MAPPING: - case OCLASS_DEFACL: - case OCLASS_EXTENSION: - case OCLASS_EVENT_TRIGGER: - case OCLASS_PARAMETER_ACL: - case OCLASS_PUBLICATION: - case OCLASS_PUBLICATION_NAMESPACE: - case OCLASS_PUBLICATION_REL: - case OCLASS_SUBSCRIPTION: - case OCLASS_TRANSFORM: - - /* - * We don't expect any of these sorts of objects to depend on - * a column. - */ - elog(ERROR, "unexpected object depending on column: %s", - getObjectDescription(&foundObject, false)); - break; - - /* - * There's intentionally no default: case here; we want the - * compiler to warn if a new OCLASS hasn't been handled above. - */ - } - } - - systable_endscan(scan); + RememberAllDependentForRebuilding(tab, rel, attnum); /* * Now scan for dependencies of this column on other things. The only * things we should find are the dependency on the column datatype and * possibly a collation dependency. Those can be removed. */ + depRel = table_open(DependRelationId, RowExclusiveLock); + ScanKeyInit(&key[0], Anum_pg_depend_classid, BTEqualStrategyNumber, F_OIDEQ, @@ -13695,6 +13493,230 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, return address; } +/* + * Subroutine for ATExecAlterColumnType: Find everything that depends on the + * column (constraints, indexes, etc), and record enough information to let us + * recreate the objects. + * + * The actual recreation does not happen here, but only after we have + * performed all the individual ALTER TYPE operations. We have to save + * the info before executing ALTER TYPE, though, else the deparser will + * get confused. + */ +static void +RememberAllDependentForRebuilding(AlteredTableInfo *tab, Relation rel, AttrNumber attnum) +{ + Relation depRel; + ScanKeyData key[3]; + SysScanDesc scan; + HeapTuple depTup; + + depRel = table_open(DependRelationId, AccessShareLock); + + ScanKeyInit(&key[0], + Anum_pg_depend_refclassid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationRelationId)); + ScanKeyInit(&key[1], + Anum_pg_depend_refobjid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + ScanKeyInit(&key[2], + Anum_pg_depend_refobjsubid, + BTEqualStrategyNumber, F_INT4EQ, + Int32GetDatum((int32) attnum)); + + scan = systable_beginscan(depRel, DependReferenceIndexId, true, + NULL, 3, key); + + while (HeapTupleIsValid(depTup = systable_getnext(scan))) + { + Form_pg_depend foundDep = (Form_pg_depend) GETSTRUCT(depTup); + ObjectAddress foundObject; + + foundObject.classId = foundDep->classid; + foundObject.objectId = foundDep->objid; + foundObject.objectSubId = foundDep->objsubid; + + switch (getObjectClass(&foundObject)) + { + case OCLASS_CLASS: + { + char relKind = get_rel_relkind(foundObject.objectId); + + if (relKind == RELKIND_INDEX || + relKind == RELKIND_PARTITIONED_INDEX) + { + Assert(foundObject.objectSubId == 0); + RememberIndexForRebuilding(foundObject.objectId, tab); + } + else if (relKind == RELKIND_SEQUENCE) + { + /* + * This must be a SERIAL column's sequence. We need + * not do anything to it. + */ + Assert(foundObject.objectSubId == 0); + } + else + { + /* Not expecting any other direct dependencies... */ + elog(ERROR, "unexpected object depending on column: %s", + getObjectDescription(&foundObject, false)); + } + break; + } + + case OCLASS_CONSTRAINT: + Assert(foundObject.objectSubId == 0); + RememberConstraintForRebuilding(foundObject.objectId, tab); + break; + + case OCLASS_REWRITE: + /* XXX someday see if we can cope with revising views */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter type of a column used by a view or rule"), + errdetail("%s depends on column \"%s\"", + getObjectDescription(&foundObject, false), + get_attname(RelationGetRelid(rel), attnum, false)))); + break; + + case OCLASS_TRIGGER: + + /* + * 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 will take a + * significant amount of new code. Since we can't easily tell + * which case applies, we punt for both. FIXME someday. + */ + 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), + get_attname(RelationGetRelid(rel), attnum, false)))); + break; + + case OCLASS_POLICY: + + /* + * A policy can depend on a column because the column is + * specified in the policy's USING or WITH CHECK qual + * expressions. It might be possible to rewrite and recheck + * the policy expression, but punt for now. It's certainly + * easy enough to remove and recreate the policy; still, FIXME + * someday. + */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter type of a column used in a policy definition"), + errdetail("%s depends on column \"%s\"", + getObjectDescription(&foundObject, false), + get_attname(RelationGetRelid(rel), attnum, false)))); + break; + + case OCLASS_DEFAULT: + { + ObjectAddress col = GetAttrDefaultColumnAddress(foundObject.objectId); + + if (col.objectId == RelationGetRelid(rel) && + col.objectSubId == attnum) + { + /* + * Ignore the column's own default expression, which + * called is supposed to deal with. + */ + Assert(build_column_default(rel, attnum)); + } + else + { + /* + * This must be a reference from the expression of a + * generated column elsewhere in the same table. + * Changing the type of a column that is used by a + * generated column is not allowed by SQL standard, so + * just punt for now. It might be doable with some + * thinking and effort. + */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot alter type of a column used by a generated column"), + errdetail("Column \"%s\" is used by generated column \"%s\".", + get_attname(RelationGetRelid(rel), attnum, false), + get_attname(col.objectId, + col.objectSubId, + false)))); + } + break; + } + + case OCLASS_STATISTIC_EXT: + + /* + * Give the extended-stats machinery a chance to fix anything + * that this column type change would break. + */ + RememberStatisticsForRebuilding(foundObject.objectId, tab); + break; + + case OCLASS_PROC: + case OCLASS_TYPE: + case OCLASS_CAST: + case OCLASS_COLLATION: + case OCLASS_CONVERSION: + case OCLASS_LANGUAGE: + case OCLASS_LARGEOBJECT: + case OCLASS_OPERATOR: + case OCLASS_OPCLASS: + case OCLASS_OPFAMILY: + case OCLASS_AM: + case OCLASS_AMOP: + case OCLASS_AMPROC: + case OCLASS_SCHEMA: + case OCLASS_TSPARSER: + case OCLASS_TSDICT: + case OCLASS_TSTEMPLATE: + case OCLASS_TSCONFIG: + case OCLASS_ROLE: + case OCLASS_ROLE_MEMBERSHIP: + case OCLASS_DATABASE: + case OCLASS_TBLSPACE: + case OCLASS_FDW: + case OCLASS_FOREIGN_SERVER: + case OCLASS_USER_MAPPING: + case OCLASS_DEFACL: + case OCLASS_EXTENSION: + case OCLASS_EVENT_TRIGGER: + case OCLASS_PARAMETER_ACL: + case OCLASS_PUBLICATION: + case OCLASS_PUBLICATION_NAMESPACE: + case OCLASS_PUBLICATION_REL: + case OCLASS_SUBSCRIPTION: + case OCLASS_TRANSFORM: + + /* + * We don't expect any of these sorts of objects to depend on + * a column. + */ + elog(ERROR, "unexpected object depending on column: %s", + getObjectDescription(&foundObject, false)); + break; + + /* + * There's intentionally no default: case here; we want the + * compiler to warn if a new OCLASS hasn't been handled above. + */ + } + } + + systable_endscan(scan); + table_close(depRel, AccessShareLock); +} + /* * Subroutine for ATExecAlterColumnType: remember that a replica identity * needs to be reset. @@ -13754,7 +13776,7 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab) /* * For the index of a constraint, if any, remember if it is used for * the table's replica identity or if it is a clustered index, so that - * ATPostAlterTypeCleanup() can queue up commands necessary to restore + * ATPostAlterColumnCleanup() can queue up commands necessary to restore * those properties. */ indoid = get_constraint_index(conoid); @@ -13808,7 +13830,7 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) /* * Remember if this index is used for the table's replica identity - * or if it is a clustered index, so that ATPostAlterTypeCleanup() + * or if it is a clustered index, so that ATPostAlterColumnCleanup() * can queue up commands necessary to restore those properties. */ RememberReplicaIdentityForRebuilding(indoid, tab); @@ -13851,7 +13873,7 @@ RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab) * queue entries to do those steps later. */ static void -ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) +ATPostAlterColumnCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) { ObjectAddress obj; ObjectAddresses *objects; @@ -13929,9 +13951,9 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) if (relid != tab->relid && contype == CONSTRAINT_FOREIGN) LockRelationOid(relid, AccessExclusiveLock); - ATPostAlterTypeParse(oldId, relid, confrelid, - (char *) lfirst(def_item), - wqueue, lockmode, tab->rewrite); + ATPostAlterColumnParse(oldId, relid, confrelid, + (char *) lfirst(def_item), + wqueue, lockmode, tab->rewrite); } forboth(oid_item, tab->changedIndexOids, def_item, tab->changedIndexDefs) @@ -13940,9 +13962,9 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) Oid relid; relid = IndexGetRelation(oldId, false); - ATPostAlterTypeParse(oldId, relid, InvalidOid, - (char *) lfirst(def_item), - wqueue, lockmode, tab->rewrite); + ATPostAlterColumnParse(oldId, relid, InvalidOid, + (char *) lfirst(def_item), + wqueue, lockmode, tab->rewrite); ObjectAddressSet(obj, RelationRelationId, oldId); add_exact_object_address(&obj, objects); @@ -13956,9 +13978,9 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) Oid relid; relid = StatisticsGetRelation(oldId, false); - ATPostAlterTypeParse(oldId, relid, InvalidOid, - (char *) lfirst(def_item), - wqueue, lockmode, tab->rewrite); + ATPostAlterColumnParse(oldId, relid, InvalidOid, + (char *) lfirst(def_item), + wqueue, lockmode, tab->rewrite); ObjectAddressSet(obj, StatisticExtRelationId, oldId); add_exact_object_address(&obj, objects); @@ -14020,8 +14042,8 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) * operator that's not available for the new column type. */ static void -ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, - List **wqueue, LOCKMODE lockmode, bool rewrite) +ATPostAlterColumnParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, + List **wqueue, LOCKMODE lockmode, bool rewrite) { List *raw_parsetree_list; List *querytree_list; @@ -14226,7 +14248,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, } /* - * Subroutine for ATPostAlterTypeParse() to recreate any existing comment + * Subroutine for ATPostAlterColumnParse() to recreate any existing comment * for a table or domain constraint that is being rebuilt. * * objid is the OID of the constraint. @@ -14276,7 +14298,7 @@ RebuildConstraintComment(AlteredTableInfo *tab, int pass, Oid objid, } /* - * Subroutine for ATPostAlterTypeParse(). Calls out to CheckIndexCompatible() + * Subroutine for ATPostAlterColumnParse(). Calls out to CheckIndexCompatible() * for the real analysis, then mutates the IndexStmt based on that verdict. */ static void @@ -14301,7 +14323,7 @@ TryReuseIndex(Oid oldId, IndexStmt *stmt) } /* - * Subroutine for ATPostAlterTypeParse(). + * Subroutine for ATPostAlterColumnParse(). * * Stash the old P-F equality operator into the Constraint node, for possible * use by ATAddForeignKeyConstraint() in determining whether revalidation of -- 2.18.0