From e6418c3f36618c517b160ab71895975773d16f6c Mon Sep 17 00:00:00 2001 From: Amul Sul Date: Wed, 22 Nov 2023 18:23:56 +0530 Subject: [PATCH v5 1/2] 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 | 474 ++++++++++++++++--------------- 1 file changed, 248 insertions(+), 226 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 323d9bf8702..ccc152f54e9 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -557,14 +557,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); @@ -5156,7 +5158,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) { @@ -13291,15 +13293,225 @@ 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. + */ + 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, + ObjectIdGetDatum(RelationRelationId)); + ScanKeyInit(&key[1], + Anum_pg_depend_objid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(RelationGetRelid(rel))); + ScanKeyInit(&key[2], + Anum_pg_depend_objsubid, + BTEqualStrategyNumber, F_INT4EQ, + Int32GetDatum((int32) attnum)); + + scan = systable_beginscan(depRel, DependDependerIndexId, true, + NULL, 3, key); + + while (HeapTupleIsValid(depTup = systable_getnext(scan))) + { + Form_pg_depend foundDep = (Form_pg_depend) GETSTRUCT(depTup); + ObjectAddress foundObject; + + foundObject.classId = foundDep->refclassid; + foundObject.objectId = foundDep->refobjid; + foundObject.objectSubId = foundDep->refobjsubid; + + if (foundDep->deptype != DEPENDENCY_NORMAL) + elog(ERROR, "found unexpected dependency type '%c'", + foundDep->deptype); + if (!(foundDep->refclassid == TypeRelationId && + foundDep->refobjid == attTup->atttypid) && + !(foundDep->refclassid == CollationRelationId && + foundDep->refobjid == attTup->attcollation)) + elog(ERROR, "found unexpected dependency for column: %s", + getObjectDescription(&foundObject, false)); + + CatalogTupleDelete(depRel, &depTup->t_self); + } + + systable_endscan(scan); + + table_close(depRel, RowExclusiveLock); + + /* + * Here we go --- change the recorded column type and collation. (Note + * heapTup is a copy of the syscache entry, so okay to scribble on.) First + * fix up the missing value if any. + */ + if (attTup->atthasmissing) + { + Datum missingval; + bool missingNull; + + /* if rewrite is true the missing value should already be cleared */ + Assert(tab->rewrite == 0); + + /* Get the missing value datum */ + missingval = heap_getattr(heapTup, + Anum_pg_attribute_attmissingval, + attrelation->rd_att, + &missingNull); + + /* if it's a null array there is nothing to do */ + + if (!missingNull) + { + /* + * Get the datum out of the array and repack it in a new array + * built with the new type data. We assume that since the table + * doesn't need rewriting, the actual Datum doesn't need to be + * changed, only the array metadata. + */ + + int one = 1; + bool isNull; + Datum valuesAtt[Natts_pg_attribute] = {0}; + bool nullsAtt[Natts_pg_attribute] = {0}; + bool replacesAtt[Natts_pg_attribute] = {0}; + HeapTuple newTup; + + missingval = array_get_element(missingval, + 1, + &one, + 0, + attTup->attlen, + attTup->attbyval, + attTup->attalign, + &isNull); + missingval = PointerGetDatum(construct_array(&missingval, + 1, + targettype, + tform->typlen, + tform->typbyval, + tform->typalign)); + + valuesAtt[Anum_pg_attribute_attmissingval - 1] = missingval; + replacesAtt[Anum_pg_attribute_attmissingval - 1] = true; + nullsAtt[Anum_pg_attribute_attmissingval - 1] = false; + + newTup = heap_modify_tuple(heapTup, RelationGetDescr(attrelation), + valuesAtt, nullsAtt, replacesAtt); + heap_freetuple(heapTup); + heapTup = newTup; + attTup = (Form_pg_attribute) GETSTRUCT(heapTup); + } + } + + attTup->atttypid = targettype; + attTup->atttypmod = targettypmod; + attTup->attcollation = targetcollid; + if (list_length(typeName->arrayBounds) > PG_INT16_MAX) + ereport(ERROR, + errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("too many array dimensions")); + attTup->attndims = list_length(typeName->arrayBounds); + attTup->attlen = tform->typlen; + attTup->attbyval = tform->typbyval; + attTup->attalign = tform->typalign; + attTup->attstorage = tform->typstorage; + attTup->attcompression = InvalidCompressionMethod; + + ReleaseSysCache(typeTuple); + + CatalogTupleUpdate(attrelation, &heapTup->t_self, heapTup); + + table_close(attrelation, RowExclusiveLock); + + /* Install dependencies on new datatype and collation */ + add_column_datatype_dependency(RelationGetRelid(rel), attnum, targettype); + add_column_collation_dependency(RelationGetRelid(rel), attnum, targetcollid); + + /* + * Drop any pg_statistic entry for the column, since it's now wrong type + */ + RemoveStatistics(RelationGetRelid(rel), attnum); + + InvokeObjectPostAlterHook(RelationRelationId, + RelationGetRelid(rel), attnum); + + /* + * Update the default, if present, by brute force --- remove and re-add + * the default. Probably unsafe to take shortcuts, since the new version + * may well have additional dependencies. (It's okay to do this now, + * rather than after other ALTER TYPE commands, since the default won't + * depend on other column types.) + */ + if (defaultexpr) + { + /* + * If it's a GENERATED default, drop its dependency records, in + * particular its INTERNAL dependency on the column, which would + * otherwise cause dependency.c to refuse to perform the deletion. + */ + if (attTup->attgenerated) + { + Oid attrdefoid = GetAttrDefaultOid(RelationGetRelid(rel), attnum); + + if (!OidIsValid(attrdefoid)) + elog(ERROR, "could not find attrdef tuple for relation %u attnum %d", + RelationGetRelid(rel), attnum); + (void) deleteDependencyRecordsFor(AttrDefaultRelationId, attrdefoid, false); + } + + /* + * Make updates-so-far visible, particularly the new pg_attribute row + * which will be updated again. + */ + CommandCounterIncrement(); + + /* + * We use RESTRICT here for safety, but at present we do not expect + * anything to depend on the default. + */ + RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, true, + true); + + StoreAttrDefault(rel, attnum, defaultexpr, true, false); + } + + ObjectAddressSubSet(address, RelationRelationId, + RelationGetRelid(rel), attnum); + + /* Cleanup */ + heap_freetuple(heapTup); + + 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, @@ -13366,7 +13578,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, errmsg("cannot alter type of a column used by a view or rule"), errdetail("%s depends on column \"%s\"", getObjectDescription(&foundObject, false), - colName))); + get_attname(RelationGetRelid(rel), attnum, false)))); break; case OCLASS_TRIGGER: @@ -13385,7 +13597,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, errmsg("cannot alter type of a column used in a trigger definition"), errdetail("%s depends on column \"%s\"", getObjectDescription(&foundObject, false), - colName))); + get_attname(RelationGetRelid(rel), attnum, false)))); break; case OCLASS_POLICY: @@ -13403,7 +13615,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, errmsg("cannot alter type of a column used in a policy definition"), errdetail("%s depends on column \"%s\"", getObjectDescription(&foundObject, false), - colName))); + get_attname(RelationGetRelid(rel), attnum, false)))); break; case OCLASS_DEFAULT: @@ -13415,9 +13627,9 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, { /* * Ignore the column's own default expression, which - * we will deal with below. + * called is supposed to deal with. */ - Assert(defaultexpr); + Assert(build_column_default(rel, attnum)); } else { @@ -13433,7 +13645,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, (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(RelationGetRelid(rel), attnum, false), get_attname(col.objectId, col.objectSubId, false)))); @@ -13501,197 +13713,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, } systable_endscan(scan); - - /* - * 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. - */ - ScanKeyInit(&key[0], - Anum_pg_depend_classid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationRelationId)); - ScanKeyInit(&key[1], - Anum_pg_depend_objid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationGetRelid(rel))); - ScanKeyInit(&key[2], - Anum_pg_depend_objsubid, - BTEqualStrategyNumber, F_INT4EQ, - Int32GetDatum((int32) attnum)); - - scan = systable_beginscan(depRel, DependDependerIndexId, true, - NULL, 3, key); - - while (HeapTupleIsValid(depTup = systable_getnext(scan))) - { - Form_pg_depend foundDep = (Form_pg_depend) GETSTRUCT(depTup); - ObjectAddress foundObject; - - foundObject.classId = foundDep->refclassid; - foundObject.objectId = foundDep->refobjid; - foundObject.objectSubId = foundDep->refobjsubid; - - if (foundDep->deptype != DEPENDENCY_NORMAL) - elog(ERROR, "found unexpected dependency type '%c'", - foundDep->deptype); - if (!(foundDep->refclassid == TypeRelationId && - foundDep->refobjid == attTup->atttypid) && - !(foundDep->refclassid == CollationRelationId && - foundDep->refobjid == attTup->attcollation)) - elog(ERROR, "found unexpected dependency for column: %s", - getObjectDescription(&foundObject, false)); - - CatalogTupleDelete(depRel, &depTup->t_self); - } - - systable_endscan(scan); - - table_close(depRel, RowExclusiveLock); - - /* - * Here we go --- change the recorded column type and collation. (Note - * heapTup is a copy of the syscache entry, so okay to scribble on.) First - * fix up the missing value if any. - */ - if (attTup->atthasmissing) - { - Datum missingval; - bool missingNull; - - /* if rewrite is true the missing value should already be cleared */ - Assert(tab->rewrite == 0); - - /* Get the missing value datum */ - missingval = heap_getattr(heapTup, - Anum_pg_attribute_attmissingval, - attrelation->rd_att, - &missingNull); - - /* if it's a null array there is nothing to do */ - - if (!missingNull) - { - /* - * Get the datum out of the array and repack it in a new array - * built with the new type data. We assume that since the table - * doesn't need rewriting, the actual Datum doesn't need to be - * changed, only the array metadata. - */ - - int one = 1; - bool isNull; - Datum valuesAtt[Natts_pg_attribute] = {0}; - bool nullsAtt[Natts_pg_attribute] = {0}; - bool replacesAtt[Natts_pg_attribute] = {0}; - HeapTuple newTup; - - missingval = array_get_element(missingval, - 1, - &one, - 0, - attTup->attlen, - attTup->attbyval, - attTup->attalign, - &isNull); - missingval = PointerGetDatum(construct_array(&missingval, - 1, - targettype, - tform->typlen, - tform->typbyval, - tform->typalign)); - - valuesAtt[Anum_pg_attribute_attmissingval - 1] = missingval; - replacesAtt[Anum_pg_attribute_attmissingval - 1] = true; - nullsAtt[Anum_pg_attribute_attmissingval - 1] = false; - - newTup = heap_modify_tuple(heapTup, RelationGetDescr(attrelation), - valuesAtt, nullsAtt, replacesAtt); - heap_freetuple(heapTup); - heapTup = newTup; - attTup = (Form_pg_attribute) GETSTRUCT(heapTup); - } - } - - attTup->atttypid = targettype; - attTup->atttypmod = targettypmod; - attTup->attcollation = targetcollid; - if (list_length(typeName->arrayBounds) > PG_INT16_MAX) - ereport(ERROR, - errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("too many array dimensions")); - attTup->attndims = list_length(typeName->arrayBounds); - attTup->attlen = tform->typlen; - attTup->attbyval = tform->typbyval; - attTup->attalign = tform->typalign; - attTup->attstorage = tform->typstorage; - attTup->attcompression = InvalidCompressionMethod; - - ReleaseSysCache(typeTuple); - - CatalogTupleUpdate(attrelation, &heapTup->t_self, heapTup); - - table_close(attrelation, RowExclusiveLock); - - /* Install dependencies on new datatype and collation */ - add_column_datatype_dependency(RelationGetRelid(rel), attnum, targettype); - add_column_collation_dependency(RelationGetRelid(rel), attnum, targetcollid); - - /* - * Drop any pg_statistic entry for the column, since it's now wrong type - */ - RemoveStatistics(RelationGetRelid(rel), attnum); - - InvokeObjectPostAlterHook(RelationRelationId, - RelationGetRelid(rel), attnum); - - /* - * Update the default, if present, by brute force --- remove and re-add - * the default. Probably unsafe to take shortcuts, since the new version - * may well have additional dependencies. (It's okay to do this now, - * rather than after other ALTER TYPE commands, since the default won't - * depend on other column types.) - */ - if (defaultexpr) - { - /* - * If it's a GENERATED default, drop its dependency records, in - * particular its INTERNAL dependency on the column, which would - * otherwise cause dependency.c to refuse to perform the deletion. - */ - if (attTup->attgenerated) - { - Oid attrdefoid = GetAttrDefaultOid(RelationGetRelid(rel), attnum); - - if (!OidIsValid(attrdefoid)) - elog(ERROR, "could not find attrdef tuple for relation %u attnum %d", - RelationGetRelid(rel), attnum); - (void) deleteDependencyRecordsFor(AttrDefaultRelationId, attrdefoid, false); - } - - /* - * Make updates-so-far visible, particularly the new pg_attribute row - * which will be updated again. - */ - CommandCounterIncrement(); - - /* - * We use RESTRICT here for safety, but at present we do not expect - * anything to depend on the default. - */ - RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, true, - true); - - StoreAttrDefault(rel, attnum, defaultexpr, true, false); - } - - ObjectAddressSubSet(address, RelationRelationId, - RelationGetRelid(rel), attnum); - - /* Cleanup */ - heap_freetuple(heapTup); - - return address; + table_close(depRel, AccessShareLock); } /* @@ -13753,7 +13775,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); @@ -13807,7 +13829,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); @@ -13850,7 +13872,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; @@ -13928,9 +13950,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) @@ -13939,9 +13961,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); @@ -13955,9 +13977,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); @@ -14019,8 +14041,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; @@ -14225,7 +14247,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. @@ -14275,7 +14297,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 @@ -14300,7 +14322,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