diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql index c4ef8e78a5..18725a02d1 100644 --- a/src/backend/catalog/information_schema.sql +++ b/src/backend/catalog/information_schema.sql @@ -514,16 +514,18 @@ CREATE VIEW column_column_usage AS CAST(ad.attname AS sql_identifier) AS dependent_column FROM pg_namespace n, pg_class c, pg_depend d, - pg_attribute ac, pg_attribute ad + pg_attribute ac, pg_attribute ad, pg_attrdef atd WHERE n.oid = c.relnamespace AND c.oid = ac.attrelid AND c.oid = ad.attrelid - AND d.classid = 'pg_catalog.pg_class'::regclass + AND ac.attnum <> ad.attnum + AND ad.attrelid = atd.adrelid + AND ad.attnum = atd.adnum + AND d.classid = 'pg_catalog.pg_attrdef'::regclass AND d.refclassid = 'pg_catalog.pg_class'::regclass - AND d.objid = d.refobjid - AND c.oid = d.objid - AND d.objsubid = ad.attnum + AND d.objid = atd.oid + AND d.refobjid = ac.attrelid AND d.refobjsubid = ac.attnum AND ad.attgenerated <> '' AND pg_has_role(c.relowner, 'USAGE'); diff --git a/src/backend/catalog/pg_attrdef.c b/src/backend/catalog/pg_attrdef.c index 490a52a086..2d63c6e62a 100644 --- a/src/backend/catalog/pg_attrdef.c +++ b/src/backend/catalog/pg_attrdef.c @@ -174,37 +174,23 @@ StoreAttrDefault(Relation rel, AttrNumber attnum, /* * Make a dependency so that the pg_attrdef entry goes away if the column - * (or whole table) is deleted. + * (or whole table) is deleted. In the case of a generated column, make + * it an internal dependency to prevent the default expression from being + * deleted separately. */ colobject.classId = RelationRelationId; colobject.objectId = RelationGetRelid(rel); colobject.objectSubId = attnum; - recordDependencyOn(&defobject, &colobject, DEPENDENCY_AUTO); + recordDependencyOn(&defobject, &colobject, + attgenerated ? DEPENDENCY_INTERNAL : DEPENDENCY_AUTO); /* * Record dependencies on objects used in the expression, too. */ - if (attgenerated) - { - /* - * Generated column: Dropping anything that the generation expression - * refers to automatically drops the generated column. - */ - recordDependencyOnSingleRelExpr(&colobject, expr, RelationGetRelid(rel), - DEPENDENCY_AUTO, - DEPENDENCY_AUTO, false); - } - else - { - /* - * Normal default: Dropping anything that the default refers to - * requires CASCADE and drops the default only. - */ - recordDependencyOnSingleRelExpr(&defobject, expr, RelationGetRelid(rel), - DEPENDENCY_NORMAL, - DEPENDENCY_NORMAL, false); - } + recordDependencyOnSingleRelExpr(&defobject, expr, RelationGetRelid(rel), + DEPENDENCY_NORMAL, + DEPENDENCY_NORMAL, false); /* * Post creation hook for attribute defaults. diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index fc3fc9b384..80faae985e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7899,6 +7899,7 @@ ATExecDropExpression(Relation rel, const char *colName, bool missing_ok, LOCKMOD Form_pg_attribute attTup; AttrNumber attnum; Relation attrelation; + Oid attrdefoid; ObjectAddress address; attrelation = table_open(AttributeRelationId, RowExclusiveLock); @@ -7936,71 +7937,44 @@ ATExecDropExpression(Relation rel, const char *colName, bool missing_ok, LOCKMOD } } + /* + * Mark the column as no longer generated. (The atthasdef flag needs to + * get cleared too, but RemoveAttrDefault will handle that.) + */ attTup->attgenerated = '\0'; CatalogTupleUpdate(attrelation, &tuple->t_self, tuple); InvokeObjectPostAlterHook(RelationRelationId, RelationGetRelid(rel), - attTup->attnum); - ObjectAddressSubSet(address, RelationRelationId, - RelationGetRelid(rel), attnum); + attnum); heap_freetuple(tuple); table_close(attrelation, RowExclusiveLock); + /* + * Drop the dependency records of the GENERATED expression, in particular + * its INTERNAL dependency on the column, which would otherwise cause + * dependency.c to refuse to perform the deletion. + */ + 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 above changes visible */ CommandCounterIncrement(); - RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, false, false); - /* - * Remove all dependencies of this (formerly generated) column on other - * columns in the same table. (See StoreAttrDefault() for which - * dependencies are created.) We don't expect there to be dependencies - * between columns of the same table for other reasons, so it's okay to - * remove all of them. + * Get rid of the GENERATED expression itself. We use RESTRICT here for + * safety, but at present we do not expect anything to depend on the + * default. */ - { - Relation depRel; - ScanKeyData key[3]; - SysScanDesc scan; - HeapTuple tup; - - 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(attnum)); - - scan = systable_beginscan(depRel, DependDependerIndexId, true, - NULL, 3, key); - - while (HeapTupleIsValid(tup = systable_getnext(scan))) - { - Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup); - - if (depform->refclassid == RelationRelationId && - depform->refobjid == RelationGetRelid(rel) && - depform->refobjsubid != 0 && - depform->deptype == DEPENDENCY_AUTO) - { - CatalogTupleDelete(depRel, &tup->t_self); - } - } - - systable_endscan(scan); - - table_close(depRel, RowExclusiveLock); - } + RemoveAttrDefault(RelationGetRelid(rel), attnum, DROP_RESTRICT, + false, false); + ObjectAddressSubSet(address, RelationRelationId, + RelationGetRelid(rel), attnum); return address; } @@ -12548,21 +12522,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, */ Assert(foundObject.objectSubId == 0); } - else if (relKind == RELKIND_RELATION && - foundObject.objectSubId != 0 && - get_attgenerated(foundObject.objectId, foundObject.objectSubId)) - { - /* - * Changing the type of a column that is used by a - * generated column is not allowed by SQL standard. It - * might be doable with some thinking and effort. - */ - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("cannot alter type of a column used by a generated column"), - errdetail("Column \"%s\" is used by generated column \"%s\".", - colName, get_attname(foundObject.objectId, foundObject.objectSubId, false)))); - } else { /* Not expecting any other direct dependencies... */ @@ -12625,13 +12584,39 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, break; case OCLASS_DEFAULT: + { + ObjectAddress col = GetAttrDefaultColumnAddress(foundObject.objectId); - /* - * Ignore the column's default expression, since we will fix - * it below. - */ - Assert(defaultexpr); - break; + 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: @@ -12694,9 +12679,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, /* * Now scan for dependencies of this column on other things. The only - * thing we should find is the dependency on the column datatype, which we - * want to remove, possibly a collation dependency, and dependencies on - * other columns if it is a generated column. + * 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, @@ -12723,18 +12707,13 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, foundObject.objectId = foundDep->refobjid; foundObject.objectSubId = foundDep->refobjsubid; - if (foundDep->deptype != DEPENDENCY_NORMAL && - foundDep->deptype != DEPENDENCY_AUTO) + 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) && - !(foundDep->refclassid == RelationRelationId && - foundDep->refobjid == RelationGetRelid(rel) && - foundDep->refobjsubid != 0) - ) + foundDep->refobjid == attTup->attcollation)) elog(ERROR, "found unexpected dependency for column: %s", getObjectDescription(&foundObject, false)); @@ -12850,7 +12829,25 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, */ if (defaultexpr) { - /* Must make new row visible since it will be updated again */ + /* + * 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(); /* diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c index d979f93b3d..1592090839 100644 --- a/src/bin/pg_dump/pg_dump_sort.c +++ b/src/bin/pg_dump/pg_dump_sort.c @@ -1203,10 +1203,10 @@ repairDependencyLoop(DumpableObject **loop, * Loop of table with itself --- just ignore it. * * (Actually, what this arises from is a dependency of a table column on - * another column, which happens with generated columns; or a dependency - * of a table column on the whole table, which happens with partitioning. - * But we didn't pay attention to sub-object IDs while collecting the - * dependency data, so we can't see that here.) + * another column, which happened with generated columns before v15; or a + * dependency of a table column on the whole table, which happens with + * partitioning. But we didn't pay attention to sub-object IDs while + * collecting the dependency data, so we can't see that here.) */ if (nLoop == 1) { diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index dcc0aba82a..1383761c1f 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202203171 +#define CATALOG_VERSION_NO 202203211 #endif diff --git a/src/test/regress/expected/generated.out b/src/test/regress/expected/generated.out index cb9373227d..bb4190340e 100644 --- a/src/test/regress/expected/generated.out +++ b/src/test/regress/expected/generated.out @@ -477,7 +477,12 @@ SELECT * FROM gtest_tableoid; -- drop column behavior CREATE TABLE gtest10 (a int PRIMARY KEY, b int, c int GENERATED ALWAYS AS (b * 2) STORED); -ALTER TABLE gtest10 DROP COLUMN b; +ALTER TABLE gtest10 DROP COLUMN b; -- fails +ERROR: cannot drop column b of table gtest10 because other objects depend on it +DETAIL: column c of table gtest10 depends on column b of table gtest10 +HINT: Use DROP ... CASCADE to drop the dependent objects too. +ALTER TABLE gtest10 DROP COLUMN b CASCADE; -- drops c too +NOTICE: drop cascades to column c of table gtest10 \d gtest10 Table "public.gtest10" Column | Type | Collation | Nullable | Default @@ -519,6 +524,10 @@ SELECT a, c FROM gtest12s; -- allowed (2 rows) RESET ROLE; +DROP FUNCTION gf1(int); -- fail +ERROR: cannot drop function gf1(integer) because other objects depend on it +DETAIL: column c of table gtest12s depends on function gf1(integer) +HINT: Use DROP ... CASCADE to drop the dependent objects too. DROP TABLE gtest11s, gtest12s; DROP FUNCTION gf1(int); DROP USER regress_user11; diff --git a/src/test/regress/sql/generated.sql b/src/test/regress/sql/generated.sql index b7eb072671..378297e6ea 100644 --- a/src/test/regress/sql/generated.sql +++ b/src/test/regress/sql/generated.sql @@ -231,7 +231,8 @@ SELECT * FROM gtest_tableoid; -- drop column behavior CREATE TABLE gtest10 (a int PRIMARY KEY, b int, c int GENERATED ALWAYS AS (b * 2) STORED); -ALTER TABLE gtest10 DROP COLUMN b; +ALTER TABLE gtest10 DROP COLUMN b; -- fails +ALTER TABLE gtest10 DROP COLUMN b CASCADE; -- drops c too \d gtest10 @@ -260,6 +261,7 @@ SELECT gf1(10); -- not allowed SELECT a, c FROM gtest12s; -- allowed RESET ROLE; +DROP FUNCTION gf1(int); -- fail DROP TABLE gtest11s, gtest12s; DROP FUNCTION gf1(int); DROP USER regress_user11;