Re: BUG #15865: ALTER TABLE statements causing "relation already exists" errors when some indexes exist

I wrote:
> As before, I attach a patch against HEAD, plus one that assumes e76de8861
> has been reverted first, which is likely easier to review.
> Unlike yesterday, I'm feeling pretty good about this patch now, but it
> still wouldn't hurt for somebody else to go over it.

I started to back-patch this, and soon noticed that the content of the
OCLASS_CONSTRAINT case branch in ATExecAlterColumnType has varied across
versions, which makes copy-and-pasting it seem pretty hazardous.  Hence
it seems prudent to do slightly more work and split that code out into
a subroutine rather than having two copies.  As attached, which is a
hopefully-final patch for HEAD.  As before, it presumes reversion of
e76de8861, because it's a lot easier to see what's going on that way.

BTW ... while working on this, I got annoyed by the fact that
ATExecAlterColumnGenericOptions was inserted, no doubt with the aid of a
dartboard, into the middle of a large group of AlterColumnType-related
functions.  Would anyone mind a separate patch to relocate it down
past those, probably just before ATExecChangeOwner?


            regards, tom lane

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 98519ef..7d62a0f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -456,6 +456,8 @@ 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);
+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,
@@ -10600,11 +10602,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);

@@ -10647,13 +10644,7 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
                         relKind == RELKIND_PARTITIONED_INDEX)
                     {
                         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));
-                        }
+                        RememberIndexForRebuilding(foundObject.objectId, tab);
                     }
                     else if (relKind == RELKIND_SEQUENCE)
                     {
@@ -10689,18 +10680,7 @@ 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);
-
-                    tab->changedConstraintOids =
-                        lappend_oid(tab->changedConstraintOids,
-                                    foundObject.objectId);
-                    tab->changedConstraintDefs =
-                        lappend(tab->changedConstraintDefs,
-                                defstring);
-                }
+                RememberConstraintForRebuilding(foundObject.objectId, tab);
                 break;

             case OCLASS_REWRITE:
@@ -11000,6 +10980,75 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
 }

 /*
+ * Subroutine for ATExecAlterColumnType: remember that a constraint needs
+ * to be rebuilt (which we might already know).
+ */
+static void
+RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab)
+{
+    /*
+     * 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);
+
+        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))
+        {
+            RememberConstraintForRebuilding(conoid, tab);
+        }
+        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
  */
 static ObjectAddress
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index cedc7c6..a639601 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1900,6 +1900,64 @@ select * from anothertab;
 (3 rows)

 drop table anothertab;
+-- 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
+--------+---------+-----------+----------+---------
+ f1     | integer |           | not null |
+ 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
+--------+--------+-----------+----------+---------
+ f1     | bigint |           | not null |
+ 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);
 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..0369909 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1317,6 +1317,31 @@ select * from anothertab;

 drop table anothertab;

+-- 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;
+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;
+
 create table another (f1 int, f2 text);

 insert into another values(1, 'one');