On 2020-02-11 00:38, Quan Zongliang wrote:
new patch attached.

I didn't like so much how the updating of the replica identity was hacked into the middle of ATRewriteCatalogs(). I have an alternative proposal in the attached patch that queues up an ALTER TABLE ... REPLICA IDENTITY command into the normal ALTER TABLE processing. I have also added tests to the test suite.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 730a5dc46ec9722aa0416e5a85987c6385e6170c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 5 Mar 2020 10:11:13 +0100
Subject: [PATCH v4] Preserve replica identity index across ALTER TABLE rewrite

---
 src/backend/commands/tablecmds.c              | 42 +++++++++++++++++
 src/backend/utils/cache/lsyscache.c           | 23 ++++++++++
 src/include/utils/lsyscache.h                 |  1 +
 .../regress/expected/replica_identity.out     | 46 +++++++++++++++++++
 src/test/regress/sql/replica_identity.sql     | 21 +++++++++
 5 files changed, 133 insertions(+)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7a13b97164..8edec9cbe7 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -176,6 +176,7 @@ typedef struct AlteredTableInfo
        List       *changedConstraintDefs;      /* string definitions of same */
        List       *changedIndexOids;   /* OIDs of indexes to rebuild */
        List       *changedIndexDefs;   /* string definitions of same */
+       char       *replicaIdentityIndex;       /* index to reset as REPLICA 
IDENTITY */
 } AlteredTableInfo;
 
 /* Struct describing one new constraint to check in Phase 3 scan */
@@ -11564,6 +11565,22 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation 
rel,
        return address;
 }
 
+/*
+ * Subroutine for ATExecAlterColumnType: remember that a replica identity
+ * needs to be reset.
+ */
+static void
+RememberReplicaIdentityForRebuilding(Oid indoid, AlteredTableInfo *tab)
+{
+       if (!get_index_is_replident(indoid))
+               return;
+
+       if (tab->replicaIdentityIndex)
+               elog(ERROR, "relation %u has multiple indexes marked as replica 
identity", tab->relid);
+
+       tab->replicaIdentityIndex = get_rel_name(indoid);
+}
+
 /*
  * Subroutine for ATExecAlterColumnType: remember that a constraint needs
  * to be rebuilt (which we might already know).
@@ -11582,11 +11599,16 @@ RememberConstraintForRebuilding(Oid conoid, 
AlteredTableInfo *tab)
        {
                /* OK, capture the constraint's existing definition string */
                char       *defstring = pg_get_constraintdef_command(conoid);
+               Oid                     indoid;
 
                tab->changedConstraintOids = 
lappend_oid(tab->changedConstraintOids,
                                                                                
                 conoid);
                tab->changedConstraintDefs = lappend(tab->changedConstraintDefs,
                                                                                
         defstring);
+
+               indoid = get_constraint_index(conoid);
+               if (OidIsValid(indoid))
+                       RememberReplicaIdentityForRebuilding(indoid, tab);
        }
 }
 
@@ -11629,6 +11651,8 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo 
*tab)
                                                                                
                indoid);
                        tab->changedIndexDefs = lappend(tab->changedIndexDefs,
                                                                                
        defstring);
+
+                       RememberReplicaIdentityForRebuilding(indoid, tab);
                }
        }
 }
@@ -11737,6 +11761,24 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo 
*tab, LOCKMODE lockmode)
                add_exact_object_address(&obj, objects);
        }
 
+       /*
+        * Queue up command to restore replica identity index marking
+        */
+       if (tab->replicaIdentityIndex)
+       {
+               AlterTableCmd *cmd = makeNode(AlterTableCmd);
+               ReplicaIdentityStmt *subcmd = makeNode(ReplicaIdentityStmt);
+
+               subcmd->identity_type = REPLICA_IDENTITY_INDEX;
+               subcmd->name = tab->replicaIdentityIndex;
+               cmd->subtype = AT_ReplicaIdentity;
+               cmd->def = (Node *) subcmd;
+
+               /* 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 3da90cb72a..f7b0c1408f 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3227,3 +3227,26 @@ get_index_column_opclass(Oid index_oid, int attno)
 
        return opclass;
 }
+
+/*
+ * get_index_is_replident
+ *
+ * Returns indisreplident field.
+ */
+bool
+get_index_is_replident(Oid index_oid)
+{
+       HeapTuple               tuple;
+       Form_pg_index   rd_index;
+       bool                    result;
+
+       tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(index_oid));
+       if (!HeapTupleIsValid(tuple))
+               return false;
+
+       rd_index = (Form_pg_index) GETSTRUCT(tuple);
+       result = rd_index->indisreplident;
+       ReleaseSysCache(tuple);
+
+       return result;
+}
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index f132d39458..ee4683843e 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -181,6 +181,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 Oid     get_index_column_opclass(Oid index_oid, int attno);
+extern bool    get_index_is_replident(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/replica_identity.out 
b/src/test/regress/expected/replica_identity.out
index 739608aab4..79002197a7 100644
--- a/src/test/regress/expected/replica_identity.out
+++ b/src/test/regress/expected/replica_identity.out
@@ -179,5 +179,51 @@ SELECT relreplident FROM pg_class WHERE oid = 
'test_replica_identity'::regclass;
  n
 (1 row)
 
+---
+-- Test that ALTER TABLE rewrite preserves nondefault replica identity
+---
+-- constraint variant
+CREATE TABLE test_replica_identity2 (id int UNIQUE NOT NULL);
+ALTER TABLE test_replica_identity2 REPLICA IDENTITY USING INDEX 
test_replica_identity2_id_key;
+\d test_replica_identity2
+       Table "public.test_replica_identity2"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           | not null | 
+Indexes:
+    "test_replica_identity2_id_key" UNIQUE CONSTRAINT, btree (id) REPLICA 
IDENTITY
+
+ALTER TABLE test_replica_identity2 ALTER COLUMN id TYPE bigint;
+\d test_replica_identity2
+      Table "public.test_replica_identity2"
+ Column |  Type  | Collation | Nullable | Default 
+--------+--------+-----------+----------+---------
+ id     | bigint |           | not null | 
+Indexes:
+    "test_replica_identity2_id_key" UNIQUE CONSTRAINT, btree (id) REPLICA 
IDENTITY
+
+-- straight index variant
+CREATE TABLE test_replica_identity3 (id int NOT NULL);
+CREATE UNIQUE INDEX test_replica_identity3_id_key ON test_replica_identity3 
(id);
+ALTER TABLE test_replica_identity3 REPLICA IDENTITY USING INDEX 
test_replica_identity3_id_key;
+\d test_replica_identity3
+       Table "public.test_replica_identity3"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ id     | integer |           | not null | 
+Indexes:
+    "test_replica_identity3_id_key" UNIQUE, btree (id) REPLICA IDENTITY
+
+ALTER TABLE test_replica_identity3 ALTER COLUMN id TYPE bigint;
+\d test_replica_identity3
+      Table "public.test_replica_identity3"
+ Column |  Type  | Collation | Nullable | Default 
+--------+--------+-----------+----------+---------
+ id     | bigint |           | not null | 
+Indexes:
+    "test_replica_identity3_id_key" UNIQUE, btree (id) REPLICA IDENTITY
+
 DROP TABLE test_replica_identity;
+DROP TABLE test_replica_identity2;
+DROP TABLE test_replica_identity3;
 DROP TABLE test_replica_identity_othertable;
diff --git a/src/test/regress/sql/replica_identity.sql 
b/src/test/regress/sql/replica_identity.sql
index b08a3623b8..a5ac8f5567 100644
--- a/src/test/regress/sql/replica_identity.sql
+++ b/src/test/regress/sql/replica_identity.sql
@@ -75,5 +75,26 @@ CREATE UNIQUE INDEX test_replica_identity_partial ON 
test_replica_identity (keya
 ALTER TABLE test_replica_identity REPLICA IDENTITY NOTHING;
 SELECT relreplident FROM pg_class WHERE oid = 
'test_replica_identity'::regclass;
 
+---
+-- Test that ALTER TABLE rewrite preserves nondefault replica identity
+---
+
+-- constraint variant
+CREATE TABLE test_replica_identity2 (id int UNIQUE NOT NULL);
+ALTER TABLE test_replica_identity2 REPLICA IDENTITY USING INDEX 
test_replica_identity2_id_key;
+\d test_replica_identity2
+ALTER TABLE test_replica_identity2 ALTER COLUMN id TYPE bigint;
+\d test_replica_identity2
+
+-- straight index variant
+CREATE TABLE test_replica_identity3 (id int NOT NULL);
+CREATE UNIQUE INDEX test_replica_identity3_id_key ON test_replica_identity3 
(id);
+ALTER TABLE test_replica_identity3 REPLICA IDENTITY USING INDEX 
test_replica_identity3_id_key;
+\d test_replica_identity3
+ALTER TABLE test_replica_identity3 ALTER COLUMN id TYPE bigint;
+\d test_replica_identity3
+
 DROP TABLE test_replica_identity;
+DROP TABLE test_replica_identity2;
+DROP TABLE test_replica_identity3;
 DROP TABLE test_replica_identity_othertable;
-- 
2.25.0

Reply via email to