From 7e48fdf35dc40261f50cff8f5f80b41db5c6d8d4 Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 20 Oct 2025 15:45:03 +0800 Subject: [PATCH v1 1/1] SET DATA TYPE cope with generation expression dependency let ALTER COLUMN SET DATA TYPE cope with generation expression dependency demo: CREATE TABLE main_table (a int, b int generated always as (a) stored); ALTER TABLE main_table ALTER COLUMN a SET DATA TYPE INT8; RROR: cannot alter type of a column used by a generated column DETAIL: Column "a" is used by generated column "b". With the attached patch, the previous error will no longer occur. discussion: https://postgr.es/m/ related discussion: https://postgr.es/m/CACJufxHZsgn3zM5g-x7YmtFGzNDnRwR07S+GYfiUs+tZ45MDDw@mail.gmail.com --- src/backend/commands/tablecmds.c | 90 +++++++++++++-- src/backend/utils/adt/ruleutils.c | 108 ++++++++++++++++++ src/include/utils/ruleutils.h | 1 + .../regress/expected/generated_stored.out | 59 ++++++++-- .../regress/expected/generated_virtual.out | 50 ++++++-- src/test/regress/sql/generated_stored.sql | 18 ++- src/test/regress/sql/generated_virtual.sql | 13 ++- 7 files changed, 301 insertions(+), 38 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index fc89352b661..2dba15c0c0e 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 *changedGenerationExprOids; /* OIDs of generation expression to rebuild */ + List *changedGenerationExprDefs; /* string definitions of same */ } AlteredTableInfo; /* Struct describing one new constraint to check in Phase 3 scan */ @@ -650,6 +652,7 @@ static void RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableT Relation rel, AttrNumber attnum, const char *colName); static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab); static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab); +static void RememberGenerationExpressionForRebuilding(Oid indoid, AlteredTableInfo *tab); static void RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab); static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode); @@ -15171,20 +15174,12 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, /* * This must be a reference from the expression of a * generated column elsewhere in the same table. - * Changing the type/generated expression 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. + * Changing the type of a column that is used by a + * generated column is not allowed by SQL standard, but + * we allow it. */ if (subtype == AT_AlterColumnType) - 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)))); + RememberGenerationExpressionForRebuilding(foundObject.objectId, tab); } break; } @@ -15367,6 +15362,32 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) } } +/* + * Subroutine for ATExecAlterColumnType: remember that a generation expression object + * needs to be rebuilt (which we might already know). + */ +static void +RememberGenerationExpressionForRebuilding(Oid attrdefoid, AlteredTableInfo *tab) +{ + /* + * This de-duplication check is critical for two independent reasons: we + * mustn't try to recreate the same generation expression twice, and if the + * generation expression 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->changedGenerationExprOids, attrdefoid)) + { + /* OK, capture the generation expression's existing definition string */ + char *defstring = pg_get_generationdef(attrdefoid); + + tab->changedGenerationExprOids = lappend_oid(tab->changedGenerationExprOids, attrdefoid); + tab->changedGenerationExprDefs = lappend(tab->changedGenerationExprDefs, defstring); + } +} + + /* * Subroutine for ATExecAlterColumnType: remember that a statistics object * needs to be rebuilt (which we might already know). @@ -15507,6 +15528,17 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) add_exact_object_address(&obj, objects); } + /* add dependencies for new generation expression */ + forboth(oid_item, tab->changedGenerationExprOids, + def_item, tab->changedGenerationExprDefs) + { + Oid oldId = lfirst_oid(oid_item); + + ATPostAlterTypeParse(oldId, tab->relid, InvalidOid, + (char *) lfirst(def_item), + wqueue, lockmode, tab->rewrite); + } + /* add dependencies for new statistics */ forboth(oid_item, tab->changedStatisticsOids, def_item, tab->changedStatisticsDefs) @@ -15582,6 +15614,29 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) * The objects will get recreated during subsequent passes over the work * queue. */ + + /* + * We have already deleted the dependent objects; now remove these objects + * themselves to avoid deleting them twice. + */ + list_free(tab->changedConstraintOids); + list_free(tab->changedConstraintDefs); + list_free(tab->changedIndexOids); + list_free(tab->changedIndexDefs); + list_free(tab->changedStatisticsOids); + list_free(tab->changedStatisticsDefs); + list_free(tab->changedGenerationExprOids); + list_free(tab->changedGenerationExprDefs); + + tab->changedConstraintOids = NIL; + tab->changedConstraintDefs = NIL; + tab->changedIndexOids = NIL; + tab->changedIndexDefs = NIL; + tab->changedStatisticsOids = NIL; + tab->changedStatisticsDefs = NIL; + tab->changedGenerationExprOids = NIL; + tab->changedGenerationExprDefs = NIL; + } /* @@ -15743,6 +15798,17 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd, else Assert(con->contype == CONSTR_NOTNULL); } + else if (cmd->subtype == AT_SetExpression) + { + /* + * The generation expression is rebuilt due to a data type + * change, but AT_PASS_ALTER_TYPE occurs earlier than + * AT_PASS_SET_EXPRESSION. Therefore, we need to prepend + * this command to the AT_PASS_SET_EXPRESSION pass. + */ + tab->subcmds[AT_PASS_SET_EXPRESSION] = + lcons(cmd, tab->subcmds[AT_PASS_SET_EXPRESSION]); + } else elog(ERROR, "unexpected statement subtype: %d", (int) cmd->subtype); diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 050eef97a4c..6f75733620e 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -25,6 +25,7 @@ #include "access/table.h" #include "catalog/pg_aggregate.h" #include "catalog/pg_am.h" +#include "catalog/pg_attrdef.h" #include "catalog/pg_authid.h" #include "catalog/pg_collation.h" #include "catalog/pg_constraint.h" @@ -367,6 +368,7 @@ static char *pg_get_partkeydef_worker(Oid relid, int prettyFlags, bool attrsOnly, bool missing_ok); static char *pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, int prettyFlags, bool missing_ok); +static char *pg_get_generationdef_worker(Oid attrdefoid, bool fullCommand, int prettyFlags, bool missing_ok); static text *pg_get_expr_worker(text *expr, Oid relid, int prettyFlags); static int print_function_arguments(StringInfo buf, HeapTuple proctup, bool print_table_args, bool print_defaults); @@ -2185,6 +2187,112 @@ pg_get_constraintdef_command(Oid constraintId) return pg_get_constraintdef_worker(constraintId, true, 0, false); } +/* + * Internal version for use by ALTER TABLE. + * Returns a palloc'd C string; no pretty-printing. + */ +char * +pg_get_generationdef(Oid attrdefoid) +{ + return pg_get_generationdef_worker(attrdefoid, true, 0, false); +} + +/* + * if fullCommand is true, return + * "ALTER TABLE %s ALTER COLUMN %s SET EXPRESSION AS ( %s ) ", + * otherwise return the text of the genereration expression + */ +static char * +pg_get_generationdef_worker(Oid attrdefoid, bool fullCommand, + int prettyFlags, bool missing_ok) +{ + HeapTuple tup; + Form_pg_attrdef atdform; + StringInfoData buf; + SysScanDesc scandesc; + ScanKeyData scankey[1]; + Relation attrdef; + Datum val; + text *result; + bool isnull; + + Snapshot snapshot = RegisterSnapshot(GetTransactionSnapshot()); + attrdef = table_open(AttrDefaultRelationId, AccessShareLock); + + ScanKeyInit(&scankey[0], + Anum_pg_attrdef_oid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(attrdefoid)); + + scandesc = systable_beginscan(attrdef, + AttrDefaultOidIndexId, + true, + snapshot, + 1, + scankey); + + /* + * We later use the tuple with SysCacheGetAttr() as if we had obtained it + * via SearchSysCache, which works fine. + */ + tup = systable_getnext(scandesc); + + UnregisterSnapshot(snapshot); + + if (!HeapTupleIsValid(tup)) + { + if (missing_ok) + { + systable_endscan(scandesc); + table_close(attrdef, AccessShareLock); + return NULL; + } + elog(ERROR, "could not find tuple for attribute defaults %u", attrdefoid); + } + + atdform = (Form_pg_attrdef) GETSTRUCT(tup); + + initStringInfo(&buf); + + if (fullCommand) + { + HeapTuple tuple; + Form_pg_attribute attrtuple; + + Assert(OidIsValid(atdform->adrelid)); + + tuple = SearchSysCacheAttNum(atdform->adrelid, atdform->adnum); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "column number %d of relation %u does not exist", + atdform->adnum, atdform->adrelid); + + attrtuple = (Form_pg_attribute) GETSTRUCT(tuple); + appendStringInfo(&buf, "ALTER TABLE %s ALTER COLUMN %s SET EXPRESSION AS ( ", + generate_qualified_relation_name(atdform->adrelid), + quote_identifier(NameStr(attrtuple->attname))); + + ReleaseSysCache(tuple); + } + + val = fastgetattr(tup, + Anum_pg_attrdef_adbin, + RelationGetDescr(attrdef), &isnull); + if (isnull) + elog(ERROR, "null adbin for rel %u", atdform->adrelid); + + result = pg_get_expr_worker(DatumGetTextPP(val), atdform->adrelid, 0); + + appendStringInfo(&buf, "%s", text_to_cstring(result)); + + if (fullCommand) + appendStringInfoChar(&buf, ')'); + + systable_endscan(scandesc); + table_close(attrdef, AccessShareLock); + + return buf.data; +} + /* * As of 9.4, we now use an MVCC snapshot for this. */ diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h index 7ba7d887914..a1d629dccfd 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_generationdef(Oid attrdefoid); #endif /* RULEUTILS_H */ diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out index adac2cedfb2..1f43f90fac1 100644 --- a/src/test/regress/expected/generated_stored.out +++ b/src/test/regress/expected/generated_stored.out @@ -1120,6 +1120,41 @@ SELECT * FROM gtest25 ORDER BY a; Indexes: "gtest25_pkey" PRIMARY KEY, btree (a) +CREATE STATISTICS gtest25_stat ON ((a + 1) is not null or (b + 1) is not null) FROM gtest25; +CREATE STATISTICS gtest25_1_stat ON ((b + 1) is not null) FROM gtest25; +CREATE INDEX gtest25_a_b_idx ON gtest25((a+b)); +CREATE INDEX gtest25_b_idx ON gtest25((b)); +ALTER TABLE gtest25 ALTER COLUMN a SET DATA TYPE BIGINT USING (a); +--change type and change generation expression together +ALTER TABLE gtest25 + ALTER COLUMN a SET DATA TYPE numeric USING (a + 1), + ALTER COLUMN b SET DATA TYPE BIGINT, + ALTER COLUMN b SET EXPRESSION AS (a * 4); +\d gtest25 + Table "generated_stored_tests.gtest25" + Column | Type | Collation | Nullable | Default +--------+------------------+-----------+----------+------------------------------------------------------ + a | numeric | | not null | + b | bigint | | | generated always as ((a * 4::numeric)) stored + c | integer | | | 42 + x | integer | | | generated always as (c * 4) stored + d | double precision | | | 101 + y | double precision | | | generated always as (d * 4::double precision) stored +Indexes: + "gtest25_pkey" PRIMARY KEY, btree (a) + "gtest25_a_b_idx" btree ((a + b::numeric)) + "gtest25_b_idx" btree (b) +Statistics objects: + "generated_stored_tests.gtest25_1_stat" ON ((b + 1) IS NOT NULL) FROM gtest25 + "generated_stored_tests.gtest25_stat" ON ((a + 1::numeric) IS NOT NULL OR (b + 1) IS NOT NULL) FROM gtest25 + +SELECT * FROM gtest25 ORDER BY a; + a | b | c | x | d | y +---+----+----+-----+-----+----- + 4 | 16 | 42 | 168 | 101 | 404 + 5 | 20 | 42 | 168 | 101 | 404 +(2 rows) + -- ALTER TABLE ... ALTER COLUMN CREATE TABLE gtest27 ( a int, @@ -1128,14 +1163,16 @@ CREATE TABLE gtest27 ( ); INSERT INTO gtest27 (a, b) VALUES (3, 7), (4, 11); ALTER TABLE gtest27 ALTER COLUMN a TYPE text; -- error -ERROR: cannot alter type of a column used by a generated column -DETAIL: Column "a" is used by generated column "x". +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 gtest27 ALTER COLUMN a TYPE int8; -- ok ALTER TABLE gtest27 ALTER COLUMN x TYPE numeric; \d gtest27 Table "generated_stored_tests.gtest27" Column | Type | Collation | Nullable | Default --------+---------+-----------+----------+-------------------------------------------- - a | integer | | | + a | bigint | | | b | integer | | | x | numeric | | | generated always as (((a + b) * 2)) stored @@ -1171,16 +1208,14 @@ ALTER TABLE gtest27 -- Ideally you could just do this, but not today (and should x change type?): ALTER TABLE gtest27 ALTER COLUMN a TYPE float8, - ALTER COLUMN b TYPE float8; -- error -ERROR: cannot alter type of a column used by a generated column -DETAIL: Column "a" is used by generated column "x". + ALTER COLUMN b TYPE float8; -- ok \d gtest27 - Table "generated_stored_tests.gtest27" - Column | Type | Collation | Nullable | Default ---------+--------+-----------+----------+------------------------------------------ - a | bigint | | | - b | bigint | | | - x | bigint | | | generated always as ((a + b) * 2) stored + Table "generated_stored_tests.gtest27" + Column | Type | Collation | Nullable | Default +--------+------------------+-----------+----------+-------------------------------------------------------------- + a | double precision | | | + b | double precision | | | + x | bigint | | | generated always as (((a + b) * 2::double precision)) stored SELECT * FROM gtest27; a | b | x diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out index c861bd36c5a..c748d4f85e9 100644 --- a/src/test/regress/expected/generated_virtual.out +++ b/src/test/regress/expected/generated_virtual.out @@ -1082,6 +1082,32 @@ SELECT * FROM gtest25 ORDER BY a; Indexes: "gtest25_pkey" PRIMARY KEY, btree (a) +ALTER TABLE gtest25 ALTER COLUMN a SET DATA TYPE BIGINT USING (a); +--change type and change generation expression together +ALTER TABLE gtest25 + ALTER COLUMN a SET DATA TYPE numeric USING (a + 1), + ALTER COLUMN b SET DATA TYPE BIGINT, + ALTER COLUMN b SET EXPRESSION AS (a * 4); +\d gtest25 + Table "generated_virtual_tests.gtest25" + Column | Type | Collation | Nullable | Default +--------+------------------+-----------+----------+----------------------------------------------- + a | numeric | | not null | + b | bigint | | | generated always as ((a * 4::numeric)) + c | integer | | | 42 + x | integer | | | generated always as (c * 4) + d | double precision | | | 101 + y | double precision | | | generated always as (d * 4::double precision) +Indexes: + "gtest25_pkey" PRIMARY KEY, btree (a) + +SELECT * FROM gtest25 ORDER BY a; + a | b | c | x | d | y +---+----+----+-----+-----+----- + 4 | 16 | 42 | 168 | 101 | 404 + 5 | 20 | 42 | 168 | 101 | 404 +(2 rows) + -- ALTER TABLE ... ALTER COLUMN CREATE TABLE gtest27 ( a int, @@ -1090,14 +1116,16 @@ CREATE TABLE gtest27 ( ); INSERT INTO gtest27 (a, b) VALUES (3, 7), (4, 11); ALTER TABLE gtest27 ALTER COLUMN a TYPE text; -- error -ERROR: cannot alter type of a column used by a generated column -DETAIL: Column "a" is used by generated column "x". +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 gtest27 ALTER COLUMN a TYPE int8; -- ok ALTER TABLE gtest27 ALTER COLUMN x TYPE numeric; \d gtest27 Table "generated_virtual_tests.gtest27" Column | Type | Collation | Nullable | Default --------+---------+-----------+----------+------------------------------------- - a | integer | | | + a | bigint | | | b | integer | | | x | numeric | | | generated always as (((a + b) * 2)) @@ -1141,16 +1169,14 @@ ALTER TABLE gtest27 -- Ideally you could just do this, but not today (and should x change type?): ALTER TABLE gtest27 ALTER COLUMN a TYPE float8, - ALTER COLUMN b TYPE float8; -- error -ERROR: cannot alter type of a column used by a generated column -DETAIL: Column "a" is used by generated column "x". + ALTER COLUMN b TYPE float8; -- ok \d gtest27 - Table "generated_virtual_tests.gtest27" - Column | Type | Collation | Nullable | Default ---------+--------+-----------+----------+----------------------------------- - a | bigint | | | - b | bigint | | | - x | bigint | | | generated always as ((a + b) * 2) + Table "generated_virtual_tests.gtest27" + Column | Type | Collation | Nullable | Default +--------+------------------+-----------+----------+------------------------------------------------------- + a | double precision | | | + b | double precision | | | + x | bigint | | | generated always as (((a + b) * 2::double precision)) SELECT * FROM gtest27; a | b | x diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql index f56fde8d4e5..77a51edeeef 100644 --- a/src/test/regress/sql/generated_stored.sql +++ b/src/test/regress/sql/generated_stored.sql @@ -517,6 +517,21 @@ ALTER TABLE gtest25 ALTER COLUMN d SET DATA TYPE float8, SELECT * FROM gtest25 ORDER BY a; \d gtest25 +CREATE STATISTICS gtest25_stat ON ((a + 1) is not null or (b + 1) is not null) FROM gtest25; +CREATE STATISTICS gtest25_1_stat ON ((b + 1) is not null) FROM gtest25; +CREATE INDEX gtest25_a_b_idx ON gtest25((a+b)); +CREATE INDEX gtest25_b_idx ON gtest25((b)); + +ALTER TABLE gtest25 ALTER COLUMN a SET DATA TYPE BIGINT USING (a); +--change type and change generation expression together +ALTER TABLE gtest25 + ALTER COLUMN a SET DATA TYPE numeric USING (a + 1), + ALTER COLUMN b SET DATA TYPE BIGINT, + ALTER COLUMN b SET EXPRESSION AS (a * 4); +\d gtest25 + +SELECT * FROM gtest25 ORDER BY a; + -- ALTER TABLE ... ALTER COLUMN CREATE TABLE gtest27 ( a int, @@ -525,6 +540,7 @@ CREATE TABLE gtest27 ( ); INSERT INTO gtest27 (a, b) VALUES (3, 7), (4, 11); ALTER TABLE gtest27 ALTER COLUMN a TYPE text; -- error +ALTER TABLE gtest27 ALTER COLUMN a TYPE int8; -- ok ALTER TABLE gtest27 ALTER COLUMN x TYPE numeric; \d gtest27 SELECT * FROM gtest27; @@ -540,7 +556,7 @@ ALTER TABLE gtest27 -- Ideally you could just do this, but not today (and should x change type?): ALTER TABLE gtest27 ALTER COLUMN a TYPE float8, - ALTER COLUMN b TYPE float8; -- error + ALTER COLUMN b TYPE float8; -- ok \d gtest27 SELECT * FROM gtest27; diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql index adfe88d74ae..b324a12bf08 100644 --- a/src/test/regress/sql/generated_virtual.sql +++ b/src/test/regress/sql/generated_virtual.sql @@ -560,6 +560,16 @@ ALTER TABLE gtest25 ALTER COLUMN d SET DATA TYPE float8, SELECT * FROM gtest25 ORDER BY a; \d gtest25 +ALTER TABLE gtest25 ALTER COLUMN a SET DATA TYPE BIGINT USING (a); +--change type and change generation expression together +ALTER TABLE gtest25 + ALTER COLUMN a SET DATA TYPE numeric USING (a + 1), + ALTER COLUMN b SET DATA TYPE BIGINT, + ALTER COLUMN b SET EXPRESSION AS (a * 4); + +\d gtest25 +SELECT * FROM gtest25 ORDER BY a; + -- ALTER TABLE ... ALTER COLUMN CREATE TABLE gtest27 ( a int, @@ -568,6 +578,7 @@ CREATE TABLE gtest27 ( ); INSERT INTO gtest27 (a, b) VALUES (3, 7), (4, 11); ALTER TABLE gtest27 ALTER COLUMN a TYPE text; -- error +ALTER TABLE gtest27 ALTER COLUMN a TYPE int8; -- ok ALTER TABLE gtest27 ALTER COLUMN x TYPE numeric; \d gtest27 SELECT * FROM gtest27; @@ -591,7 +602,7 @@ ALTER TABLE gtest27 -- Ideally you could just do this, but not today (and should x change type?): ALTER TABLE gtest27 ALTER COLUMN a TYPE float8, - ALTER COLUMN b TYPE float8; -- error + ALTER COLUMN b TYPE float8; -- ok \d gtest27 SELECT * FROM gtest27; -- 2.34.1