]> granicus.if.org Git - postgresql/commitdiff
Fix ALTER COLUMN TYPE failure with a partial exclusion constraint.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 12 Jun 2019 16:29:24 +0000 (12:29 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 12 Jun 2019 16:29:39 +0000 (12:29 -0400)
ATExecAlterColumnType failed to consider the possibility that an index
that needs to be rebuilt might be a child of a constraint that needs to be
rebuilt.  We missed this so far because usually a constraint index doesn't
have a direct dependency on its table, just on the constraint object.
But if there's a WHERE clause, then dependency analysis of the WHERE
clause results in direct dependencies on the column(s) mentioned in WHERE.
This led to trying to drop and rebuild both the constraint and its
underlying index.

In v11/HEAD, we successfully drop both the index and the constraint,
and then try to rebuild both, and of course the second rebuild hits a
duplicate-index-name problem.  Before v11, it fails with obscure messages
about a missing relation OID, due to trying to drop the index twice.

This is essentially the same kind of problem noted in commit
20bef2c31: the possible dependency linkages are broader than what
ATExecAlterColumnType was designed for.  It was probably OK when
written, but it's certainly been broken since the introduction of
partial exclusion constraints.  Fix by adding an explicit check
for whether any of the indexes-to-be-rebuilt belong to any of the
constraints-to-be-rebuilt, and ignoring any that do.

In passing, fix a latent bug introduced by commit 8b08f7d48: in
get_constraint_index() we must "continue" not "break" when rejecting
a relation of a wrong relkind.  This is harmless today because we don't
expect that code path to be taken anyway; but if there ever were any
relations to be ignored, the existing coding would have an extremely
undesirable dependency on the order of pg_depend entries.

Also adjust a couple of obsolete comments.

Per bug #15835 from Yaroslav Schekin.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/15835-32d9b7a76c06a7a9@postgresql.org

src/backend/catalog/pg_depend.c
src/backend/commands/tablecmds.c
src/test/regress/expected/alter_table.out
src/test/regress/sql/alter_table.sql

index f7caedcc02cbf5c51bf6fd630f9d6c13c3570b4a..4116e93b64ba2ecdc8204ae0e61b80d8a5469937 100644 (file)
@@ -779,8 +779,8 @@ getOwnedSequence(Oid relid, AttrNumber attnum)
 
 /*
  * get_constraint_index
- *             Given the OID of a unique or primary-key constraint, return the
- *             OID of the underlying unique index.
+ *             Given the OID of a unique, primary-key, or exclusion constraint,
+ *             return the OID of the underlying index.
  *
  * Return InvalidOid if the index couldn't be found; this suggests the
  * given OID is bogus, but we leave it to caller to decide what to do.
@@ -827,10 +827,13 @@ get_constraint_index(Oid constraintId)
                {
                        char            relkind = get_rel_relkind(deprec->objid);
 
-                       /* This is pure paranoia; there shouldn't be any such */
+                       /*
+                        * This is pure paranoia; there shouldn't be any other relkinds
+                        * dependent on a constraint.
+                        */
                        if (relkind != RELKIND_INDEX &&
                                relkind != RELKIND_PARTITIONED_INDEX)
-                               break;
+                               continue;
 
                        indexId = deprec->objid;
                        break;
@@ -845,8 +848,9 @@ get_constraint_index(Oid constraintId)
 
 /*
  * get_index_constraint
- *             Given the OID of an index, return the OID of the owning unique or
- *             primary-key constraint, or InvalidOid if no such constraint.
+ *             Given the OID of an index, return the OID of the owning unique,
+ *             primary-key, or exclusion constraint, or InvalidOid if there
+ *             is no owning constraint.
  */
 Oid
 get_index_constraint(Oid indexId)
index 98519ef836cc9e1afb624757ec9fa10ca2f2c734..cb2c5e181b3067158aff65e3c4a0f2ca715a6b74 100644 (file)
@@ -10508,6 +10508,9 @@ 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
@@ -10646,14 +10649,20 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
                                        if (relKind == RELKIND_INDEX ||
                                                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.
+                                                */
                                                Assert(foundObject.objectSubId == 0);
-                                               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));
-                                               }
+                                               tab->changedIndexOids =
+                                                       list_append_unique_oid(tab->changedIndexOids,
+                                                                                                  foundObject.objectId);
                                        }
                                        else if (relKind == RELKIND_SEQUENCE)
                                        {
@@ -10819,6 +10828,41 @@ 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
index cedc7c689a711365622063e169bb4ad7ca9e1aa0..c845a16d557a16d2f697b2de0d842e8446327f83 100644 (file)
@@ -1899,6 +1899,46 @@ select * from anothertab;
  f      | IT WAS NULL!
 (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);
+alter table anothertab
+  add exclude using btree (f3 with =);
+alter table anothertab
+  add exclude using btree (f4 with =) where (f4 is not null);
+\d anothertab
+             Table "public.anothertab"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ f1     | integer |           | not null | 
+ f2     | integer |           |          | 
+ f3     | integer |           |          | 
+ f4     | integer |           |          | 
+Indexes:
+    "anothertab_pkey" PRIMARY KEY, btree (f1)
+    "anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+    "anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
+    "anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
+
+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;
+\d anothertab
+            Table "public.anothertab"
+ Column |  Type  | Collation | Nullable | Default 
+--------+--------+-----------+----------+---------
+ f1     | bigint |           | not null | 
+ f2     | bigint |           |          | 
+ f3     | bigint |           |          | 
+ f4     | bigint |           |          | 
+Indexes:
+    "anothertab_pkey" PRIMARY KEY, btree (f1)
+    "anothertab_f2_key" UNIQUE CONSTRAINT, btree (f2)
+    "anothertab_f3_excl" EXCLUDE USING btree (f3 WITH =)
+    "anothertab_f4_excl" EXCLUDE USING btree (f4 WITH =) WHERE (f4 IS NOT NULL)
+
 drop table anothertab;
 create table another (f1 int, f2 text);
 insert into another values(1, 'one');
index e8e094ab777ec7a80cfecfe4f779c971797e62de..8c8b627ad6f4f08c0317a2cbbc95d4c39f40ef2d 100644 (file)
@@ -1317,6 +1317,23 @@ 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);
+alter table anothertab
+  add exclude using btree (f3 with =);
+alter table anothertab
+  add exclude using btree (f4 with =) where (f4 is not null);
+
+\d anothertab
+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;
+\d anothertab
+
+drop table anothertab;
+
 create table another (f1 int, f2 text);
 
 insert into another values(1, 'one');