diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 8ce12272cf..8f058604e8 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -415,6 +415,9 @@ 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 RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab, + DependencyType deptype); +static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab); static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char *colName, List *options, LOCKMODE lockmode); static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, @@ -9021,9 +9024,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, SysScanDesc scan; HeapTuple depTup; ObjectAddress address; - ListCell *lc; - ListCell *prev; - ListCell *next; attrelation = heap_open(AttributeRelationId, RowExclusiveLock); @@ -9092,11 +9092,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, * performed all the individual ALTER TYPE operations. We have to save * the info before executing ALTER TYPE, though, else the deparser will * get confused. - * - * There could be multiple entries for the same object, so we must check - * to ensure we process each one only once. Note: we assume that an index - * that implements a constraint will not show a direct dependency on the - * column. */ depRel = heap_open(DependRelationId, RowExclusiveLock); @@ -9137,20 +9132,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, if (relKind == RELKIND_INDEX) { - /* - * Indexes that are directly dependent on the table - * might be regular indexes or constraint indexes. - * Constraint indexes typically have only indirect - * dependencies; but there are exceptions, notably - * partial exclusion constraints. Hence we must check - * whether the index depends on any constraint that's - * due to be rebuilt, which we'll do below after we've - * found all such constraints. - */ Assert(foundObject.objectSubId == 0); - tab->changedIndexOids = - list_append_unique_oid(tab->changedIndexOids, - foundObject.objectId); + RememberIndexForRebuilding(foundObject.objectId, tab); } else if (relKind == RELKIND_SEQUENCE) { @@ -9171,39 +9154,8 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, case OCLASS_CONSTRAINT: Assert(foundObject.objectSubId == 0); - if (!list_member_oid(tab->changedConstraintOids, - foundObject.objectId)) - { - char *defstring = pg_get_constraintdef_command(foundObject.objectId); - - /* - * Put NORMAL dependencies at the front of the list and - * AUTO dependencies at the back. This makes sure that - * foreign-key constraints depending on this column will - * be dropped before unique or primary-key constraints of - * the column; which we must have because the FK - * constraints depend on the indexes belonging to the - * unique constraints. - */ - if (foundDep->deptype == DEPENDENCY_NORMAL) - { - tab->changedConstraintOids = - lcons_oid(foundObject.objectId, - tab->changedConstraintOids); - tab->changedConstraintDefs = - lcons(defstring, - tab->changedConstraintDefs); - } - else - { - tab->changedConstraintOids = - lappend_oid(tab->changedConstraintOids, - foundObject.objectId); - tab->changedConstraintDefs = - lappend(tab->changedConstraintDefs, - defstring); - } - } + RememberConstraintForRebuilding(foundObject.objectId, tab, + foundDep->deptype); break; case OCLASS_REWRITE: @@ -9322,41 +9274,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, systable_endscan(scan); - /* - * Check the collected index OIDs to see which ones belong to the - * constraint(s) of the table, and drop those from the list of indexes - * that we need to process; rebuilding the constraints will handle them. - */ - prev = NULL; - for (lc = list_head(tab->changedIndexOids); lc; lc = next) - { - Oid indexoid = lfirst_oid(lc); - Oid conoid; - - next = lnext(lc); - - conoid = get_index_constraint(indexoid); - if (OidIsValid(conoid) && - list_member_oid(tab->changedConstraintOids, conoid)) - tab->changedIndexOids = list_delete_cell(tab->changedIndexOids, - lc, prev); - else - prev = lc; - } - - /* - * Now collect the definitions of the indexes that must be rebuilt. (We - * could merge this into the previous loop, but it'd be more complicated - * for little gain.) - */ - foreach(lc, tab->changedIndexOids) - { - Oid indexoid = lfirst_oid(lc); - - tab->changedIndexDefs = lappend(tab->changedIndexDefs, - pg_get_indexdef_string(indexoid)); - } - /* * 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 @@ -9460,6 +9377,95 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, return address; } +/* + * Subroutine for ATExecAlterColumnType: remember that a constraint needs + * to be rebuilt (which we might already know). + */ +static void +RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab, + DependencyType deptype) +{ + /* + * This de-duplication check is critical for two independent reasons: we + * mustn't try to recreate the same constraint twice, and if a constraint + * depends on more than one column whose type is to be altered, we must + * capture its definition string before applying any of the column type + * changes. ruleutils.c will get confused if we ask again later. + */ + if (!list_member_oid(tab->changedConstraintOids, conoid)) + { + /* OK, capture the constraint's existing definition string */ + char *defstring = pg_get_constraintdef_command(conoid); + + /* + * Put NORMAL dependencies at the front of the list and AUTO + * dependencies at the back. This makes sure that foreign-key + * constraints depending on this column will be dropped before unique + * or primary-key constraints of the column; which we must have + * because the FK constraints depend on the indexes belonging to the + * unique constraints. + */ + if (deptype == DEPENDENCY_NORMAL) + { + tab->changedConstraintOids = lcons_oid(conoid, + tab->changedConstraintOids); + tab->changedConstraintDefs = lcons(defstring, + tab->changedConstraintDefs); + } + else + { + tab->changedConstraintOids = lappend_oid(tab->changedConstraintOids, + conoid); + tab->changedConstraintDefs = lappend(tab->changedConstraintDefs, + defstring); + } + } +} + +/* + * Subroutine for ATExecAlterColumnType: remember that an index needs + * to be rebuilt (which we might already know). + */ +static void +RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab) +{ + /* + * This de-duplication check is critical for two independent reasons: we + * mustn't try to recreate the same index twice, and if an index depends + * on more than one column whose type is to be altered, we must capture + * its definition string before applying any of the column type changes. + * ruleutils.c will get confused if we ask again later. + */ + if (!list_member_oid(tab->changedIndexOids, indoid)) + { + /* + * Before adding it as an index-to-rebuild, we'd better see if it + * belongs to a constraint, and if so rebuild the constraint instead. + * Typically this check fails, because constraint indexes normally + * have only dependencies on their constraint. But it's possible for + * such an index to also have direct dependencies on table columns, + * for example with a partial exclusion constraint. + */ + Oid conoid = get_index_constraint(indoid); + + if (OidIsValid(conoid)) + { + /* index dependencies on columns should generally be AUTO */ + RememberConstraintForRebuilding(conoid, tab, DEPENDENCY_AUTO); + } + else + { + /* OK, capture the index's existing definition string */ + char *defstring = pg_get_indexdef_string(indoid); + + tab->changedIndexOids = lappend_oid(tab->changedIndexOids, + indoid); + tab->changedIndexDefs = lappend(tab->changedIndexDefs, + defstring); + } + } +} + /* * Returns the address of the modified column */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 3b69fb02e5..39e20c83e8 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1934,12 +1934,19 @@ select * from anothertab; (3 rows) drop table anothertab; --- Test alter table column type with constraint indexes (cf. bug #15835) -create table anothertab(f1 int primary key, f2 int unique, f3 int, f4 int); +-- Test index handling in alter table column type (cf. bugs #15835, #15865) +create table anothertab(f1 int primary key, f2 int unique, + f3 int, f4 int, f5 int); alter table anothertab add exclude using btree (f3 with =); alter table anothertab add exclude using btree (f4 with =) where (f4 is not null); +alter table anothertab + add exclude using btree (f4 with =) where (f5 > 0); +alter table anothertab + add unique(f1,f4); +create index on anothertab(f2,f3); +create unique index on anothertab(f4); \d anothertab Table "public.anothertab" Column | Type | Collation | Nullable | Default @@ -1948,17 +1955,23 @@ alter table anothertab f2 | integer | | | f3 | integer | | | f4 | integer | | | + f5 | integer | | | Indexes: "anothertab_pkey" PRIMARY KEY, btree (f1) + "anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4) "anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2) + "anothertab_f4_idx" UNIQUE, btree (f4) + "anothertab_f2_f3_idx" btree (f2, f3) "anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =) "anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL) + "anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0) alter table anothertab alter column f1 type bigint; alter table anothertab alter column f2 type bigint, alter column f3 type bigint, alter column f4 type bigint; +alter table anothertab alter column f5 type bigint; \d anothertab Table "public.anothertab" Column | Type | Collation | Nullable | Default @@ -1967,11 +1980,16 @@ alter table anothertab f2 | bigint | | | f3 | bigint | | | f4 | bigint | | | + f5 | bigint | | | Indexes: "anothertab_pkey" PRIMARY KEY, btree (f1) + "anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4) "anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2) + "anothertab_f4_idx" UNIQUE, btree (f4) + "anothertab_f2_f3_idx" btree (f2, f3) "anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =) "anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL) + "anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0) drop table anothertab; create table another (f1 int, f2 text); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 9e4dd8ad3d..7193bf948c 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1307,12 +1307,19 @@ select * from anothertab; drop table anothertab; --- Test alter table column type with constraint indexes (cf. bug #15835) -create table anothertab(f1 int primary key, f2 int unique, f3 int, f4 int); +-- Test index handling in alter table column type (cf. bugs #15835, #15865) +create table anothertab(f1 int primary key, f2 int unique, + f3 int, f4 int, f5 int); alter table anothertab add exclude using btree (f3 with =); alter table anothertab add exclude using btree (f4 with =) where (f4 is not null); +alter table anothertab + add exclude using btree (f4 with =) where (f5 > 0); +alter table anothertab + add unique(f1,f4); +create index on anothertab(f2,f3); +create unique index on anothertab(f4); \d anothertab alter table anothertab alter column f1 type bigint; @@ -1320,6 +1327,7 @@ alter table anothertab alter column f2 type bigint, alter column f3 type bigint, alter column f4 type bigint; +alter table anothertab alter column f5 type bigint; \d anothertab drop table anothertab;