fix-alter-table-some-more.patch
text/x-diff
Filename: fix-alter-table-some-more.patch
Type: text/x-diff
Part: 0
Patch
Format: unified
| File | + | − |
|---|---|---|
| src/backend/commands/tablecmds.c | 36 | 54 |
| src/test/regress/expected/alter_table.out | 10 | 0 |
| src/test/regress/sql/alter_table.sql | 4 | 0 |
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cb2c5e1..83dc6df 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10508,9 +10508,6 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
SysScanDesc scan;
HeapTuple depTup;
ObjectAddress address;
- ListCell *lc;
- ListCell *prev;
- ListCell *next;
/*
* Clear all the missing values if we're rewriting the table, since this
@@ -10603,11 +10600,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 = table_open(DependRelationId, RowExclusiveLock);
@@ -10650,19 +10642,21 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
relKind == RELKIND_PARTITIONED_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.
+ * This de-duplication check is critical for two
+ * independent reasons: we mustn't try to create 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.
*/
Assert(foundObject.objectSubId == 0);
- tab->changedIndexOids =
- list_append_unique_oid(tab->changedIndexOids,
- foundObject.objectId);
+ if (!list_member_oid(tab->changedIndexOids, foundObject.objectId))
+ {
+ tab->changedIndexOids = lappend_oid(tab->changedIndexOids,
+ foundObject.objectId);
+ tab->changedIndexDefs = lappend(tab->changedIndexDefs,
+ pg_get_indexdef_string(foundObject.objectId));
+ }
}
else if (relKind == RELKIND_SEQUENCE)
{
@@ -10697,6 +10691,12 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
}
case OCLASS_CONSTRAINT:
+
+ /*
+ * As with indexes, de-duplication is critical, and it's
+ * needed because the same constraint could depend on multiple
+ * target columns.
+ */
Assert(foundObject.objectSubId == 0);
if (!list_member_oid(tab->changedConstraintOids,
foundObject.objectId))
@@ -10829,41 +10829,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
* want to remove, possibly a collation dependency, and dependencies on
@@ -11242,8 +11207,25 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
def_item, tab->changedIndexDefs)
{
Oid oldId = lfirst_oid(oid_item);
+ Oid conoid;
Oid relid;
+ /*
+ * We must check each index to see if it belongs to a constraint that
+ * we already processed above. Typically this check does not fire
+ * because constraint indexes normally have only dependencies on their
+ * constraint, and thus would not have been found by the dependency
+ * scan in ATExecAlterColumnType. But it's possible for such an index
+ * to also have direct dependencies on the table, for example with a
+ * partial exclusion constraint. In that case we'd have list entries
+ * for both the index and the constraint, and we must disregard the
+ * one for the index.
+ */
+ conoid = get_index_constraint(oldId);
+ if (OidIsValid(conoid) &&
+ list_member_oid(tab->changedConstraintOids, conoid))
+ continue;
+
relid = IndexGetRelation(oldId, false);
ATPostAlterTypeParse(oldId, relid, InvalidOid,
(char *) lfirst(def_item),
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index c845a16..01ba41c 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1906,6 +1906,10 @@ 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 unique(f3,f4);
+create index on anothertab(f2,f3);
+create unique index on anothertab(f4);
\d anothertab
Table "public.anothertab"
Column | Type | Collation | Nullable | Default
@@ -1917,6 +1921,9 @@ alter table anothertab
Indexes:
"anothertab_pkey" PRIMARY KEY, btree (f1)
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+ "anothertab_f3_f4_key" UNIQUE CONSTRAINT, btree (f3, f4)
+ "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)
@@ -1936,6 +1943,9 @@ alter table anothertab
Indexes:
"anothertab_pkey" PRIMARY KEY, btree (f1)
"anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+ "anothertab_f3_f4_key" UNIQUE CONSTRAINT, btree (f3, f4)
+ "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)
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 8c8b627..95fa1ce 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1323,6 +1323,10 @@ 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 unique(f3,f4);
+create index on anothertab(f2,f3);
+create unique index on anothertab(f4);
\d anothertab
alter table anothertab alter column f1 type bigint;