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.GI13621@telsasoft.com
Backpatch-through: 9.5

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e99e16efbc06a4044f0b66fe8117c912c004c957..375e035f4e58accef1981cfb57a7aca9d3a9805c 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -174,6 +174,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 */
@@ -9437,6 +9438,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).
@@ -9481,9 +9497,18 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab,
                                                                                                 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);
+               }
        }
 }
 
@@ -9528,7 +9553,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);
                }
        }
 }
@@ -9739,6 +9770,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);
+       }
+
        /*
         * Now we can drop the existing constraints and indexes --- constraints
         * first, since some of them might depend on the indexes.  In fact, we

diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index db62daf77eccb425661ec9b9b575b982ccbd6eb5..6a6336c8619cd727790b332451f9feb077a1c9b3 100644 (file)
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3163,3 +3163,26 @@ get_index_isreplident(Oid index_oid)
 
        return result;
 }
+
+/*
+ * 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 770972a2818340f70aca69b7f132a38bce3141c7..3e545bdbb226e4ab1f84715be0009c2b48f5a65b 100644 (file)
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -179,6 +179,7 @@ extern char *get_namespace_name_or_temp(Oid nspid);
 extern Oid     get_range_subtype(Oid rangeOid);
 extern Oid     get_range_collation(Oid rangeOid);
 extern bool    get_index_isreplident(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 dc6c07f3a5e3dab78b74f1ab4cebfe32578177b8..eabd3e160cd1dd0561792eb0483aa37e61b54957 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3788,3 +3788,51 @@ alter table at_test_sql_partop attach partition at_test_sql_partop_1 for values
 drop table at_test_sql_partop;
 drop operator class at_test_sql_partop using btree;
 drop function at_test_sql_partop;
+-- 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 7193bf948ceccf71c6cbcc56eec5d161217a66c5..8afdf0613e807e6bb05d485c9b2f0fa086a77b7e 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2463,3 +2463,28 @@ alter table at_test_sql_partop attach partition at_test_sql_partop_1 for values
 drop table at_test_sql_partop;
 drop operator class at_test_sql_partop using btree;
 drop function at_test_sql_partop;
+
+-- 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;