Preserve clustered index after rewrites with ALTER TABLE

A table rewritten by ALTER TABLE would lose tracking of an index usable
for CLUSTER.  This setting is tracked by pg_index.indisclustered and is
controlled by ALTER TABLE, so some extra work was needed to restore it
properly.  Note that ALTER TABLE only marks the index that can be used
for clustering, and does not do the actual operation.

Author: Amit Langote, Justin Pryzby
Reviewed-by: Ibrar Ahmed, Michael Paquier
Discussion: https://postgr.es/m/20200202161718[email protected]
Backpatch-through: 9.5

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c23e6af7c14464191a8eef7fc9ab7899bfd9679a..6162fb018c72f212bd8495115f2e7778afffc837 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -177,6 +177,7 @@ typedef struct AlteredTableInfo
        List       *changedIndexOids;   /* OIDs of indexes to rebuild */
        List       *changedIndexDefs;   /* string definitions of same */
        char       *replicaIdentityIndex;       /* index to reset as REPLICA IDENTITY */
+       char       *clusterOnIndex;     /* index to use for CLUSTER */
 } AlteredTableInfo;
 
 /* Struct describing one new constraint to check in Phase 3 scan */
@@ -11581,6 +11582,21 @@ RememberReplicaIdentityForRebuilding(Oid indoid, AlteredTableInfo *tab)
        tab->replicaIdentityIndex = get_rel_name(indoid);
 }
 
+/*
+ * Subroutine for ATExecAlterColumnType: remember any clustered index.
+ */
+static void
+RememberClusterOnForRebuilding(Oid indoid, AlteredTableInfo *tab)
+{
+       if (!get_index_isclustered(indoid))
+               return;
+
+       if (tab->clusterOnIndex)
+               elog(ERROR, "relation %u has multiple clustered indexes", tab->relid);
+
+       tab->clusterOnIndex = get_rel_name(indoid);
+}
+
 /*
  * Subroutine for ATExecAlterColumnType: remember that a constraint needs
  * to be rebuilt (which we might already know).
@@ -11606,9 +11622,18 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab)
                tab->changedConstraintDefs = lappend(tab->changedConstraintDefs,
                                                                                         defstring);
 
+               /*
+                * For the index of a constraint, if any, remember if it is used for
+                * the table's replica identity or if it is a clustered index, so that
+                * ATPostAlterTypeCleanup() can queue up commands necessary to restore
+                * those properties.
+                */
                indoid = get_constraint_index(conoid);
                if (OidIsValid(indoid))
+               {
                        RememberReplicaIdentityForRebuilding(indoid, tab);
+                       RememberClusterOnForRebuilding(indoid, tab);
+               }
        }
 }
 
@@ -11652,7 +11677,13 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab)
                        tab->changedIndexDefs = lappend(tab->changedIndexDefs,
                                                                                        defstring);
 
+                       /*
+                        * Remember if this index is used for the table's replica identity
+                        * or if it is a clustered index, so that ATPostAlterTypeCleanup()
+                        * can queue up commands necessary to restore those properties.
+                        */
                        RememberReplicaIdentityForRebuilding(indoid, tab);
+                       RememberClusterOnForRebuilding(indoid, tab);
                }
        }
 }
@@ -11779,6 +11810,21 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
                        lappend(tab->subcmds[AT_PASS_OLD_CONSTR], cmd);
        }
 
+       /*
+        * Queue up command to restore marking of index used for cluster.
+        */
+       if (tab->clusterOnIndex)
+       {
+               AlterTableCmd *cmd = makeNode(AlterTableCmd);
+
+               cmd->subtype = AT_ClusterOn;
+               cmd->name = tab->clusterOnIndex;
+
+               /* do it after indexes and constraints */
+               tab->subcmds[AT_PASS_OLD_CONSTR] =
+                       lappend(tab->subcmds[AT_PASS_OLD_CONSTR], cmd);
+       }
+
        /*
         * It should be okay to use DROP_RESTRICT here, since nothing else should
         * be depending on these objects.

diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 0a6db0d478e3d4ad6e11d6e31bbdba43ecf49a83..a7d63f107f252037f3d059010efc6217d6fedcfc 100644 (file)
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3334,3 +3334,26 @@ get_index_isvalid(Oid index_oid)
 
        return isvalid;
 }
+
+/*
+ * get_index_isclustered
+ *
+ *             Given the index OID, return pg_index.indisclustered.
+ */
+bool
+get_index_isclustered(Oid index_oid)
+{
+       bool            isclustered;
+       HeapTuple       tuple;
+       Form_pg_index rd_index;
+
+       tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(index_oid));
+       if (!HeapTupleIsValid(tuple))
+               elog(ERROR, "cache lookup failed for index %u", index_oid);
+
+       rd_index = (Form_pg_index) GETSTRUCT(tuple);
+       isclustered = rd_index->indisclustered;
+       ReleaseSysCache(tuple);
+
+       return isclustered;
+}

diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index 374f57fb43a4f10c440de2ece5e0462c268fac14..c9c68e2f4f8af1e77612f6643e6c7b7c52e3642e 100644 (file)
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -185,6 +185,7 @@ extern Oid  get_range_collation(Oid rangeOid);
 extern Oid     get_index_column_opclass(Oid index_oid, int attno);
 extern bool    get_index_isreplident(Oid index_oid);
 extern bool get_index_isvalid(Oid index_oid);
+extern bool get_index_isclustered(Oid index_oid);
 
 #define type_is_array(typid)  (get_element_type(typid) != InvalidOid)
 /* type_is_array_domain accepts both plain arrays and domains over arrays */

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 7c2181ac2f7e7d408bc3f44b77febfee9a492f5c..e46775afbf52d4b2cdbd60e23f49842a1ab30e3f 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4302,3 +4302,51 @@ create trigger xtrig
 update bar1 set a = a + 1;
 INFO:  a=1, b=1
 /* End test case for bug #16242 */
+-- Test that ALTER TABLE rewrite preserves a clustered index
+-- for normal indexes and indexes on constraints.
+create table alttype_cluster (a int);
+alter table alttype_cluster add primary key (a);
+create index alttype_cluster_ind on alttype_cluster (a);
+alter table alttype_cluster cluster on alttype_cluster_ind;
+-- Normal index remains clustered.
+select indexrelid::regclass, indisclustered from pg_index
+  where indrelid = 'alttype_cluster'::regclass
+  order by indexrelid::regclass::text;
+      indexrelid      | indisclustered 
+----------------------+----------------
+ alttype_cluster_ind  | t
+ alttype_cluster_pkey | f
+(2 rows)
+
+alter table alttype_cluster alter a type bigint;
+select indexrelid::regclass, indisclustered from pg_index
+  where indrelid = 'alttype_cluster'::regclass
+  order by indexrelid::regclass::text;
+      indexrelid      | indisclustered 
+----------------------+----------------
+ alttype_cluster_ind  | t
+ alttype_cluster_pkey | f
+(2 rows)
+
+-- Constraint index remains clustered.
+alter table alttype_cluster cluster on alttype_cluster_pkey;
+select indexrelid::regclass, indisclustered from pg_index
+  where indrelid = 'alttype_cluster'::regclass
+  order by indexrelid::regclass::text;
+      indexrelid      | indisclustered 
+----------------------+----------------
+ alttype_cluster_ind  | f
+ alttype_cluster_pkey | t
+(2 rows)
+
+alter table alttype_cluster alter a type int;
+select indexrelid::regclass, indisclustered from pg_index
+  where indrelid = 'alttype_cluster'::regclass
+  order by indexrelid::regclass::text;
+      indexrelid      | indisclustered 
+----------------------+----------------
+ alttype_cluster_ind  | f
+ alttype_cluster_pkey | t
+(2 rows)
+
+drop table alttype_cluster;

diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 1b1315f316dabfcca697bb0c5e200257334df2a6..8eca21823d6c8a57be4fa824cb7657fe101aa423 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2847,3 +2847,28 @@ create trigger xtrig
 update bar1 set a = a + 1;
 
 /* End test case for bug #16242 */
+
+-- Test that ALTER TABLE rewrite preserves a clustered index
+-- for normal indexes and indexes on constraints.
+create table alttype_cluster (a int);
+alter table alttype_cluster add primary key (a);
+create index alttype_cluster_ind on alttype_cluster (a);
+alter table alttype_cluster cluster on alttype_cluster_ind;
+-- Normal index remains clustered.
+select indexrelid::regclass, indisclustered from pg_index
+  where indrelid = 'alttype_cluster'::regclass
+  order by indexrelid::regclass::text;
+alter table alttype_cluster alter a type bigint;
+select indexrelid::regclass, indisclustered from pg_index
+  where indrelid = 'alttype_cluster'::regclass
+  order by indexrelid::regclass::text;
+-- Constraint index remains clustered.
+alter table alttype_cluster cluster on alttype_cluster_pkey;
+select indexrelid::regclass, indisclustered from pg_index
+  where indrelid = 'alttype_cluster'::regclass
+  order by indexrelid::regclass::text;
+alter table alttype_cluster alter a type int;
+select indexrelid::regclass, indisclustered from pg_index
+  where indrelid = 'alttype_cluster'::regclass
+  order by indexrelid::regclass::text;
+drop table alttype_cluster;