Weird index ordering in psql's \d (was Re: BUG #15865: ALTER TABLE statements causing "relation already exists" errors when some indexes exist)

I wrote:
>> BTW, has anyone got an explanation for the order in which psql is
>> listing the indexes of "anothertab" in this test case?

> Ah, here's the explanation: describe.c is sorting the indexes
> with this:
>     "ORDER BY i.indisprimary DESC, i.indisunique DESC, c2.relname;"
> I can see the point of putting the pkey first, I guess, but the preference
> for uniques second seems pretty bizarre, especially since
> (a) it doesn't distinguish unique constraints from plain unique indexes and
> (b) there's no similar preference for exclusion constraints, even though
> those might be morally equivalent to a unique constraint.
> What do people think of dropping the indisunique sort column here?
> Obviously not back-patch material, but it might be more sensible
> behavior going forward.

Here's a proposed patch that does this.  The changes it causes in the
existing regression test results seem to be sufficient illustration,
so I didn't add new tests.  There is of course no documentation
touching on this point ...

With the patch, psql's rule for listing indexes is "pkey first, then
everything else in name order".  The traditional rule is basically
crazytown IMO when you consider mixes of unique constraints and plain
(non-constraint-syntax) indexes and exclusion constraints.

A different idea that might make it slightly less crazytown is to
include exclusion constraints in the secondary preference group, along
the lines of
"ORDER BY i.indisprimary DESC, i.indisunique|i.indisexclusion DESC, c2.relname;"
This'd restore what I think was the original design intention, that
the secondary preference group includes all indexes that impose
constraints on what the table can hold.  But this'd be doubling down
on what I think is fundamentally not a very good idea, so I didn't
pursue it.

Alternatively we could go further and drop the pkey preference too,
making it pure index name order, but I don't feel a need to do that.

Thoughts?

            regards, tom lane

diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 97167d2..1c770b4 100644
*** a/src/bin/psql/describe.c
--- b/src/bin/psql/describe.c
*************** describeOneTableDetails(const char *sche
*** 2312,2318 ****
                                       "  LEFT JOIN pg_catalog.pg_constraint con ON (conrelid = i.indrelid AND conindid
=i.indexrelid AND contype IN ('p','u','x'))\n"); 
              appendPQExpBuffer(&buf,
                                "WHERE c.oid = '%s' AND c.oid = i.indrelid AND i.indexrelid = c2.oid\n"
!                               "ORDER BY i.indisprimary DESC, i.indisunique DESC, c2.relname;",
                                oid);
              result = PSQLexec(buf.data);
              if (!result)
--- 2312,2318 ----
                                       "  LEFT JOIN pg_catalog.pg_constraint con ON (conrelid = i.indrelid AND conindid
=i.indexrelid AND contype IN ('p','u','x'))\n"); 
              appendPQExpBuffer(&buf,
                                "WHERE c.oid = '%s' AND c.oid = i.indrelid AND i.indexrelid = c2.oid\n"
!                               "ORDER BY i.indisprimary DESC, c2.relname;",
                                oid);
              result = PSQLexec(buf.data);
              if (!result)
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index a639601..7d077ae 100644
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
*************** create unique index on anothertab(f4);
*** 1925,1936 ****
  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
--- 1925,1936 ----
  Indexes:
      "anothertab_pkey" PRIMARY KEY, btree (f1)
      "anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
      "anothertab_f2_f3_idx" btree (f2, f3)
+     "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)
      "anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
+     "anothertab_f4_idx" UNIQUE, btree (f4)

  alter table anothertab alter column f1 type bigint;
  alter table anothertab
*************** alter table anothertab alter column f5 t
*** 1950,1961 ****
  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);
--- 1950,1961 ----
  Indexes:
      "anothertab_pkey" PRIMARY KEY, btree (f1)
      "anothertab_f1_f4_key" UNIQUE CONSTRAINT, btree (f1, f4)
      "anothertab_f2_f3_idx" btree (f2, f3)
+     "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)
      "anothertab_f4_excl1" EXCLUDE USING btree (f4 WITH =) WHERE (f5 > 0)
+     "anothertab_f4_idx" UNIQUE, btree (f4)

  drop table anothertab;
  create table another (f1 int, f2 text);
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 5305b53..9305649 100644
*** a/src/test/regress/expected/create_index.out
--- b/src/test/regress/expected/create_index.out
*************** VACUUM FULL concur_heap;
*** 1374,1383 ****
   f1     | text |           |          |
   f2     | text |           |          |
  Indexes:
-     "concur_index2" UNIQUE, btree (f1)
-     "concur_index3" UNIQUE, btree (f2) INVALID
      "concur_heap_expr_idx" btree ((f2 || f1))
      "concur_index1" btree (f2, f1)
      "concur_index4" btree (f2) WHERE f1 = 'a'::text
      "concur_index5" btree (f2) WHERE f1 = 'x'::text
      "std_index" btree (f2)
--- 1374,1383 ----
   f1     | text |           |          |
   f2     | text |           |          |
  Indexes:
      "concur_heap_expr_idx" btree ((f2 || f1))
      "concur_index1" btree (f2, f1)
+     "concur_index2" UNIQUE, btree (f1)
+     "concur_index3" UNIQUE, btree (f2) INVALID
      "concur_index4" btree (f2) WHERE f1 = 'a'::text
      "concur_index5" btree (f2) WHERE f1 = 'x'::text
      "std_index" btree (f2)
*************** REINDEX TABLE concur_heap;
*** 1390,1399 ****
   f1     | text |           |          |
   f2     | text |           |          |
  Indexes:
-     "concur_index2" UNIQUE, btree (f1)
-     "concur_index3" UNIQUE, btree (f2)
      "concur_heap_expr_idx" btree ((f2 || f1))
      "concur_index1" btree (f2, f1)
      "concur_index4" btree (f2) WHERE f1 = 'a'::text
      "concur_index5" btree (f2) WHERE f1 = 'x'::text
      "std_index" btree (f2)
--- 1390,1399 ----
   f1     | text |           |          |
   f2     | text |           |          |
  Indexes:
      "concur_heap_expr_idx" btree ((f2 || f1))
      "concur_index1" btree (f2, f1)
+     "concur_index2" UNIQUE, btree (f1)
+     "concur_index3" UNIQUE, btree (f2)
      "concur_index4" btree (f2) WHERE f1 = 'a'::text
      "concur_index5" btree (f2) WHERE f1 = 'x'::text
      "std_index" btree (f2)
*************** WARNING:  cannot reindex system catalogs
*** 2115,2122 ****
   c2     | text    |           |          |
  Indexes:
      "concur_reindex_ind1" PRIMARY KEY, btree (c1)
-     "concur_reindex_ind3" UNIQUE, btree (abs(c1))
      "concur_reindex_ind2" btree (c2)
      "concur_reindex_ind4" btree (c1, c1, c2)
  Referenced by:
      TABLE "concur_reindex_tab2" CONSTRAINT "concur_reindex_tab2_c1_fkey" FOREIGN KEY (c1) REFERENCES
concur_reindex_tab(c1)
--- 2115,2122 ----
   c2     | text    |           |          |
  Indexes:
      "concur_reindex_ind1" PRIMARY KEY, btree (c1)
      "concur_reindex_ind2" btree (c2)
+     "concur_reindex_ind3" UNIQUE, btree (abs(c1))
      "concur_reindex_ind4" btree (c1, c1, c2)
  Referenced by:
      TABLE "concur_reindex_tab2" CONSTRAINT "concur_reindex_tab2_c1_fkey" FOREIGN KEY (c1) REFERENCES
concur_reindex_tab(c1)
diff --git a/src/test/regress/expected/replica_identity.out b/src/test/regress/expected/replica_identity.out
index 175ecd2..739608a 100644
*** a/src/test/regress/expected/replica_identity.out
--- b/src/test/regress/expected/replica_identity.out
*************** SELECT relreplident FROM pg_class WHERE
*** 85,97 ****
  Indexes:
      "test_replica_identity_pkey" PRIMARY KEY, btree (id) REPLICA IDENTITY
      "test_replica_identity_expr" UNIQUE, btree (keya, keyb, (3))
      "test_replica_identity_keyab_key" UNIQUE, btree (keya, keyb)
      "test_replica_identity_nonkey" UNIQUE, btree (keya, nonkey)
      "test_replica_identity_partial" UNIQUE, btree (keya, keyb) WHERE keyb <> '3'::text
      "test_replica_identity_unique_defer" UNIQUE CONSTRAINT, btree (keya, keyb) DEFERRABLE
      "test_replica_identity_unique_nondefer" UNIQUE CONSTRAINT, btree (keya, keyb)
-     "test_replica_identity_hash" hash (nonkey)
-     "test_replica_identity_keyab" btree (keya, keyb)

  -- succeed, nondeferrable unique constraint over nonnullable cols
  ALTER TABLE test_replica_identity REPLICA IDENTITY USING INDEX test_replica_identity_unique_nondefer;
--- 85,97 ----
  Indexes:
      "test_replica_identity_pkey" PRIMARY KEY, btree (id) REPLICA IDENTITY
      "test_replica_identity_expr" UNIQUE, btree (keya, keyb, (3))
+     "test_replica_identity_hash" hash (nonkey)
+     "test_replica_identity_keyab" btree (keya, keyb)
      "test_replica_identity_keyab_key" UNIQUE, btree (keya, keyb)
      "test_replica_identity_nonkey" UNIQUE, btree (keya, nonkey)
      "test_replica_identity_partial" UNIQUE, btree (keya, keyb) WHERE keyb <> '3'::text
      "test_replica_identity_unique_defer" UNIQUE CONSTRAINT, btree (keya, keyb) DEFERRABLE
      "test_replica_identity_unique_nondefer" UNIQUE CONSTRAINT, btree (keya, keyb)

  -- succeed, nondeferrable unique constraint over nonnullable cols
  ALTER TABLE test_replica_identity REPLICA IDENTITY USING INDEX test_replica_identity_unique_nondefer;
*************** SELECT relreplident FROM pg_class WHERE
*** 115,127 ****
  Indexes:
      "test_replica_identity_pkey" PRIMARY KEY, btree (id)
      "test_replica_identity_expr" UNIQUE, btree (keya, keyb, (3))
      "test_replica_identity_keyab_key" UNIQUE, btree (keya, keyb) REPLICA IDENTITY
      "test_replica_identity_nonkey" UNIQUE, btree (keya, nonkey)
      "test_replica_identity_partial" UNIQUE, btree (keya, keyb) WHERE keyb <> '3'::text
      "test_replica_identity_unique_defer" UNIQUE CONSTRAINT, btree (keya, keyb) DEFERRABLE
      "test_replica_identity_unique_nondefer" UNIQUE CONSTRAINT, btree (keya, keyb)
-     "test_replica_identity_hash" hash (nonkey)
-     "test_replica_identity_keyab" btree (keya, keyb)

  SELECT count(*) FROM pg_index WHERE indrelid = 'test_replica_identity'::regclass AND indisreplident;
   count
--- 115,127 ----
  Indexes:
      "test_replica_identity_pkey" PRIMARY KEY, btree (id)
      "test_replica_identity_expr" UNIQUE, btree (keya, keyb, (3))
+     "test_replica_identity_hash" hash (nonkey)
+     "test_replica_identity_keyab" btree (keya, keyb)
      "test_replica_identity_keyab_key" UNIQUE, btree (keya, keyb) REPLICA IDENTITY
      "test_replica_identity_nonkey" UNIQUE, btree (keya, nonkey)
      "test_replica_identity_partial" UNIQUE, btree (keya, keyb) WHERE keyb <> '3'::text
      "test_replica_identity_unique_defer" UNIQUE CONSTRAINT, btree (keya, keyb) DEFERRABLE
      "test_replica_identity_unique_nondefer" UNIQUE CONSTRAINT, btree (keya, keyb)

  SELECT count(*) FROM pg_index WHERE indrelid = 'test_replica_identity'::regclass AND indisreplident;
   count
*************** SELECT relreplident FROM pg_class WHERE
*** 163,175 ****
  Indexes:
      "test_replica_identity_pkey" PRIMARY KEY, btree (id)
      "test_replica_identity_expr" UNIQUE, btree (keya, keyb, (3))
      "test_replica_identity_keyab_key" UNIQUE, btree (keya, keyb)
      "test_replica_identity_nonkey" UNIQUE, btree (keya, nonkey)
      "test_replica_identity_partial" UNIQUE, btree (keya, keyb) WHERE keyb <> '3'::text
      "test_replica_identity_unique_defer" UNIQUE CONSTRAINT, btree (keya, keyb) DEFERRABLE
      "test_replica_identity_unique_nondefer" UNIQUE CONSTRAINT, btree (keya, keyb)
-     "test_replica_identity_hash" hash (nonkey)
-     "test_replica_identity_keyab" btree (keya, keyb)
  Replica Identity: FULL

  ALTER TABLE test_replica_identity REPLICA IDENTITY NOTHING;
--- 163,175 ----
  Indexes:
      "test_replica_identity_pkey" PRIMARY KEY, btree (id)
      "test_replica_identity_expr" UNIQUE, btree (keya, keyb, (3))
+     "test_replica_identity_hash" hash (nonkey)
+     "test_replica_identity_keyab" btree (keya, keyb)
      "test_replica_identity_keyab_key" UNIQUE, btree (keya, keyb)
      "test_replica_identity_nonkey" UNIQUE, btree (keya, nonkey)
      "test_replica_identity_partial" UNIQUE, btree (keya, keyb) WHERE keyb <> '3'::text
      "test_replica_identity_unique_defer" UNIQUE CONSTRAINT, btree (keya, keyb) DEFERRABLE
      "test_replica_identity_unique_nondefer" UNIQUE CONSTRAINT, btree (keya, keyb)
  Replica Identity: FULL

  ALTER TABLE test_replica_identity REPLICA IDENTITY NOTHING;