Re: Change a constraint's index - ALTER TABLE ... ALTER CONSTRAINT ... USING INDEX ...

On Mon, 2020-08-10 at 09:29 +0300, Anna Akenteva wrote:
> On 2020-07-07 01:08, Tom Lane wrote:
> 
> > Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > > On 2020-Jul-05, Anna Akenteva wrote:
> > > > -- Swapping primary key's index for an equivalent index,
> > > > -- but with INCLUDE-d attributes.
> > > > CREATE UNIQUE INDEX new_idx ON target_tbl (id) INCLUDE (info);
> > > > ALTER TABLE target_tbl ALTER CONSTRAINT target_tbl_pkey USING INDEX
> > > > new_idx;
> > > > ALTER TABLE referencing_tbl ALTER CONSTRAINT 
> > > > referencing_tbl_id_ref_fkey
> > > > USING INDEX new_idx;
> > > How is this state represented by pg_dump?
> > Even if it's possible to represent, I think we should flat out reject
> > this "feature".  Primary keys that aren't primary keys don't seem like
> > a good idea.  For one thing, it won't be possible to describe the
> > constraint accurately in the information_schema.
> 
> 
> Do you think it could still be a good idea if we only swap the 
> relfilenodes of indexes, as it was suggested in [1]? The original use 
> case was getting rid of index bloat, which is now solved by REINDEX 
> CONCURRENTLY, but this feature still has its own use case of adding 
> INCLUDE-d columns to constraint indexes.

How can you just swap the filenodes if "indnatts" and "indkey" is
different, since one index has an INCLUDE clause?

I think that the original proposal is better, except that foreign key
dependencies should be changed along with the primary or unique index,
so that everything is consistent once the command is done.

Then the ALTER CONSTRAINT from that replaces the index referenced
by a foreign key becomes unnecessary and should be removed.

The value I see in this is:
- replacing a primary key index
- replacing the index behind a constraint targeted by a foreign key

Some code comments:

+   <varlistentry>
+    <term><literal>ALTER CONSTRAINT</literal> <replaceable class="parameter">constraint_name</replaceable> [USING
INDEX<replaceable class="para>
 
+    <listitem>
+     <para>
+      For uniqueness, primary key, and exclusion constraints, this form
+      replaces the original index and renames the constraint accordingly.

You forgot to mention foreign keys.

+   /* This function might need modificatoins if pg_index gets new fields */
+   Assert(Natts_pg_index == 20);

Typo.

+   if (!equal(RelationGetIndexExpressions(oldIndex),
+              RelationGetIndexExpressions(newIndex)))
+       return "Indexes must have the same non-column attributes";

Correct me if I am wrong, but constraint indexes can never use
expressions.  So this should be covered by comparing the key
attributes above (they would be 0 for an expression).

+   if (!equal(oldPredicate, newPredicate))
+   {
+       if (oldPredicate && newPredicate)
+           return "Indexes must have the same partial index predicates";
+       else
+           return "Either none or both indexes must have partial index predicates";
+   }

A constraint index can never have predicates.  Only the new index would
have to be checked.

+/*
+ * ALTER TABLE ALTER CONSTRAINT USING INDEX
+ *
+ * Replace an index of a constraint.
+ *
+ * Currently only works for UNIQUE, EXCLUSION and PRIMARY constraints.

You forgot foreign key constraints (although I think they should not be allowed).


I'll set the commitfest entry to "waiting for author".

Yours,
Laurenz Albe