From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Quan Zongliang <quanzongliang@foxmail.com>
To: Euler Taveira <euler@timbira.com.br>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Restore replication settings when modifying a field type
Date: Thu, 5 Mar 2020 13:45:24 +0100
Message-ID: <40e3fb7b-6362-6438-03d1-6b7f9dc0f962@2ndquadrant.com> (raw)
In-Reply-To: <0b190c18-1ab0-22ab-f249-af58ee967807@foxmail.com>
References: <c70fcab2-4866-0d9f-1d01-e75e189db342@gmail.com>
<20191028.133939.559712139043653003.horikyota.ntt@gmail.com>
<CAHE3wgh-JX+L_RBNQ-_SLjMNbXT4K6A5FpkLKsEmEO5mCgsUTQ@mail.gmail.com>
<61d38b0e-0831-d206-8390-2a08bb51905d@2ndquadrant.com>
<ec12720a-b834-05d4-f7fb-ab69bcce7112@foxmail.com>
<0b190c18-1ab0-22ab-f249-af58ee967807@foxmail.com>
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
Attachments:v4-0001-Preserve-replica-identity-index-across-ALTER-TABL.patch (text/plain, 8.5K) download
From 730a5dc46ec9722aa0416e5a85987c6385e6170c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@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.cindex 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.cindex 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.hindex 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.outindex 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.sqlindex 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 instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@2ndquadrant.com, quanzongliang@foxmail.com, euler@timbira.com.br, horikyota.ntt@gmail.com
Subject: Re: Restore replication settings when modifying a field type
In-Reply-To: <40e3fb7b-6362-6438-03d1-6b7f9dc0f962@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox