Re: BUG #15835: Errors altering data type of the column used in partial exclusion constraint

PG Bug reporting form <noreply@postgresql.org> writes:
> Executing the below code:

> CREATE TABLE t(some_id int);
> ALTER TABLE t ADD EXCLUDE USING btree(some_id WITH =) WHERE (some_id IS NOT
> NULL);
> ALTER TABLE t ALTER COLUMN some_id TYPE bigint;

> Produces the following errors (per PostgreSQL version):
> 9.4.19, 9.5.15, 9.6.13: ERROR:  could not open relation with OID 195837
> 10.8: ERROR: cache lookup failed for relation 630589
> 11.3, 12beta1: ERROR: relation "t_some_id_excl" already exists

Thanks for the report!  The problem seems to be that ATExecAlterColumnType
doesn't consider the possibility that an index that needs to be rebuilt
might be a child of a constraint that needs to be rebuilt.  We haven't
noticed this before because usually a constraint index doesn't have a
direct dependency on the table.  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.

In 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 even earlier than
that, because we first drop all the constraints and then drop all the
indexes.  (Commit 20bef2c31 explains the change in behavior.)

The attached patch seems to fix it in HEAD.  I'm pretty sure it will
fix the older branches too, but haven't tried to back-patch yet.

In passing, I cleaned up some obsolete comments in pg_depend.c,
and changed a "paranoia" test so that it won't break things so
badly if it fires.

            regards, tom lane

diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index f7caedc..4116e93 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -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)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 98519ef..cb2c5e1 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -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)
                     {
@@ -10820,6 +10829,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
      * want to remove, possibly a collation dependency, and dependencies on
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index cedc7c6..c845a16 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1900,6 +1900,46 @@ 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);
+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');
 insert into another values(2, 'two');
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index e8e094a..8c8b627 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -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');