Restore replication settings when modifying a field type
Started by Quan Zongliangabout 6 years ago11 messages
When the user modifies the REPLICA IDENTIFY field type, the logical
replication settings are lost.
For example:
postgres=# \d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
col1 | integer | | | | plain |
|
col2 | integer | | not null | | plain |
|
Indexes:
"t1_col2_key" UNIQUE CONSTRAINT, btree (col2) REPLICA IDENTITY
postgres=# alter table t1 alter col2 type smallint;
ALTER TABLE
postgres=# \d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
--------+----------+-----------+----------+---------+---------+--------------+-------------
col1 | integer | | | | plain |
|
col2 | smallint | | not null | | plain |
|
Indexes:
"t1_col2_key" UNIQUE CONSTRAINT, btree (col2)
In fact, the replication property of the table has not been modified,
and it is still 'i'(REPLICA_IDENTITY_INDEX). But the previously
specified index property 'indisreplident' is set to false because of the
rebuild.
So I developed a patch. If the user modifies the field type. The
associated index is REPLICA IDENTITY. Rebuild and restore replication
settings.
Regards,
Quan Zongliang
Attachments:
replidfieldtype.difftext/plain; charset=UTF-8; name=replidfieldtype.diff; x-mac-creator=0; x-mac-type=0Download
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b53f6ed3ac..c21372fe51 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -175,6 +175,8 @@ typedef struct AlteredTableInfo
List *changedConstraintDefs; /* string definitions of same */
List *changedIndexOids; /* OIDs of indexes to rebuild */
List *changedIndexDefs; /* string definitions of same */
+ Oid changedReplIdentOid; /* OID of index to reset REPLICA IDENTIFY */
+ char *changedReplIdentDef; /* string definitions of same */
} AlteredTableInfo;
/* Struct describing one new constraint to check in Phase 3 scan */
@@ -428,6 +430,7 @@ static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
AlterTableCmd *cmd, LOCKMODE lockmode);
static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab);
static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab);
+static void RememberReplicaIdentForRebuilding(Oid indoid, AlteredTableInfo *tab);
static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char *colName,
List *options, LOCKMODE lockmode);
static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab,
@@ -9991,6 +9994,9 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
{
Assert(foundObject.objectSubId == 0);
RememberIndexForRebuilding(foundObject.objectId, tab);
+
+ if (RelationGetForm(rel)->relreplident==REPLICA_IDENTITY_INDEX)
+ RememberReplicaIdentForRebuilding(foundObject.objectId, tab);
}
else if (relKind == RELKIND_SEQUENCE)
{
@@ -10010,8 +10016,14 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
}
case OCLASS_CONSTRAINT:
- Assert(foundObject.objectSubId == 0);
- RememberConstraintForRebuilding(foundObject.objectId, tab);
+ {
+ Oid indId;
+ Assert(foundObject.objectSubId == 0);
+ RememberConstraintForRebuilding(foundObject.objectId, tab);
+ indId = get_constraint_index(foundObject.objectId);
+ if (OidIsValid(indId))
+ RememberReplicaIdentForRebuilding(indId, tab);
+ }
break;
case OCLASS_REWRITE:
@@ -10324,6 +10336,36 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab)
}
}
+/*
+ * Subroutine for ATExecAlterColumnType: remember that a replica identify
+ * needs to be reset (which we might already know).
+ */
+static void
+RememberReplicaIdentForRebuilding(Oid indoid, AlteredTableInfo *tab)
+{
+ char *defstring;
+
+ /*
+ * This de-duplication check is critical for two independent reasons: we
+ * mustn't try to recreate the same constraint twice, and if a constraint
+ * depends on more than one column whose type is to be altered, we must
+ * capture its definition string before applying any of the column type
+ * changes. ruleutils.c will get confused if we ask again later.
+ */
+ if (OidIsValid(tab->changedReplIdentOid))
+ return;
+
+ /* OK, capture the constraint's existing definition string */
+ defstring = pg_get_replidentdef_command(indoid);
+
+ /* not a replica identify */
+ if (defstring==NULL)
+ return;
+
+ tab->changedReplIdentOid = indoid;
+ tab->changedReplIdentDef = defstring;
+}
+
/*
* Subroutine for ATExecAlterColumnType: remember that an index needs
* to be rebuilt (which we might already know).
@@ -10576,6 +10618,13 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
ObjectAddressSet(obj, RelationRelationId, oldId);
add_exact_object_address(&obj, objects);
}
+ if (OidIsValid(tab->changedReplIdentOid))
+ {
+ Oid relid = IndexGetRelation(tab->changedReplIdentOid, false);
+ ATPostAlterTypeParse(InvalidOid, relid, InvalidOid,
+ tab->changedReplIdentDef,
+ wqueue, lockmode, tab->rewrite);
+ }
/*
* It should be okay to use DROP_RESTRICT here, since nothing else should
@@ -10717,6 +10766,11 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
NIL,
con->conname);
}
+ else if (cmd->subtype == AT_ReplicaIdentity)
+ {
+ tab->subcmds[AT_PASS_MISC] =
+ lappend(tab->subcmds[AT_PASS_MISC], cmd);
+ }
else
elog(ERROR, "unexpected statement subtype: %d",
(int) cmd->subtype);
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 364e465cbe..13a0221973 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1855,6 +1855,38 @@ pg_get_constraintdef_ext(PG_FUNCTION_ARGS)
PG_RETURN_TEXT_P(string_to_text(res));
}
+char *pg_get_replidentdef_command(Oid indId)
+{
+ Oid relId;
+ HeapTuple ht_idx;
+ Form_pg_index idxrec;
+
+ StringInfoData buf;
+
+ ht_idx = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indId));
+ if (!HeapTupleIsValid(ht_idx))
+ elog(ERROR, "cache lookup failed for index %u", indId);
+
+ idxrec = (Form_pg_index) GETSTRUCT(ht_idx);
+ /* not a replica identify */
+ if (!idxrec->indisreplident)
+ {
+ ReleaseSysCache(ht_idx);
+ return NULL;
+ }
+ relId = idxrec->indrelid;
+ ReleaseSysCache(ht_idx);
+
+ initStringInfo(&buf);
+ appendStringInfoString(&buf, "ALTER TABLE ");
+
+ appendStringInfoString(&buf, generate_qualified_relation_name(relId));
+ appendStringInfoString(&buf, " REPLICA IDENTITY USING INDEX ");
+ appendStringInfoString(&buf, quote_identifier(get_rel_name(indId)));
+
+ return buf.data;
+}
+
/*
* Internal version that returns a full ALTER TABLE ... ADD CONSTRAINT command
*/
diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h
index 9f9b029ab8..c4a48e2336 100644
--- a/src/include/utils/ruleutils.h
+++ b/src/include/utils/ruleutils.h
@@ -23,6 +23,8 @@ extern char *pg_get_indexdef_columns(Oid indexrelid, bool pretty);
extern char *pg_get_partkeydef_columns(Oid relid, bool pretty);
+extern char *pg_get_replidentdef_command(Oid indId);
+
extern char *pg_get_constraintdef_command(Oid constraintId);
extern char *deparse_expression(Node *expr, List *dpcontext,
bool forceprefix, bool showimplicit);
Re: Restore replication settings when modifying a field type
Hello.
# The patch no longer applies on the current master. Needs a rebasing.
At Sat, 26 Oct 2019 16:50:48 +0800, Quan Zongliang <quanzongliang@gmail.com> wrote in
In fact, the replication property of the table has not been modified,
and it is still 'i'(REPLICA_IDENTITY_INDEX). But the previously
specified index property 'indisreplident' is set to false because of
the rebuild.
I suppose that the behavior is intended. Change of column types on the
publisher side can break the agreement on replica identity with
subscribers. Thus replica identity setting cannot be restored
unconditionally. For (somewhat artifitial :p) example:
P=# create table t (c1 integer, c2 text unique not null);
P=# alter table t replica identity using index t_c2_key;
P=# create publication p1 for table t;
P=# insert into t values (0, '00'), (1, '01');
S=# create table t (c1 integer, c2 text unique not null);
S=# alter table t replica identity using index t_c2_key;
S=# create subscription s1 connection '...' publication p1;
Your patch allows change of the type of c2 into integer.
P=# alter table t alter column c2 type integer using c2::integer;
P=# update t set c1 = c1 + 1 where c2 = '01';
This change doesn't affect perhaps as expected.
S=# select * from t;
c1 | c2
----+----
0 | 00
1 | 01
(2 rows)
So I developed a patch. If the user modifies the field type. The
associated index is REPLICA IDENTITY. Rebuild and restore replication
settings.
Explicit setting of replica identity premises that they are sure that
the setting works correctly. Implicit rebuilding after a type change
can silently break it.
At least we need to guarantee that the restored replica identity
setting is truly compatible with all existing subscribers. I'm not
sure about potential subscribers..
Anyway I think it is a problem that replica identity setting is
dropped silently. Perhaps a message something like "REPLICA IDENTITY
setting is lost, please redefine after confirmation of compatibility
with subscribers." is needed.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Re: Restore replication settings when modifying a field type
On 2019/10/28 12:39, Kyotaro Horiguchi wrote:
Hello.
# The patch no longer applies on the current master. Needs a rebasing.
At Sat, 26 Oct 2019 16:50:48 +0800, Quan Zongliang <quanzongliang@gmail.com> wrote in
In fact, the replication property of the table has not been modified,
and it is still 'i'(REPLICA_IDENTITY_INDEX). But the previously
specified index property 'indisreplident' is set to false because of
the rebuild.I suppose that the behavior is intended. Change of column types on the
publisher side can break the agreement on replica identity with
subscribers. Thus replica identity setting cannot be restored
unconditionally. For (somewhat artifitial :p) example:P=# create table t (c1 integer, c2 text unique not null);
P=# alter table t replica identity using index t_c2_key;
P=# create publication p1 for table t;
P=# insert into t values (0, '00'), (1, '01');
S=# create table t (c1 integer, c2 text unique not null);
S=# alter table t replica identity using index t_c2_key;
S=# create subscription s1 connection '...' publication p1;Your patch allows change of the type of c2 into integer.
P=# alter table t alter column c2 type integer using c2::integer;
P=# update t set c1 = c1 + 1 where c2 = '01';This change doesn't affect perhaps as expected.
S=# select * from t;
c1 | c2
----+----
0 | 00
1 | 01
(2 rows)So I developed a patch. If the user modifies the field type. The
associated index is REPLICA IDENTITY. Rebuild and restore replication
settings.Explicit setting of replica identity premises that they are sure that
the setting works correctly. Implicit rebuilding after a type change
can silently break it.At least we need to guarantee that the restored replica identity
setting is truly compatible with all existing subscribers. I'm not
sure about potential subscribers..Anyway I think it is a problem that replica identity setting is
dropped silently. Perhaps a message something like "REPLICA IDENTITY
setting is lost, please redefine after confirmation of compatibility
with subscribers." is needed.
In fact, the scene we encountered is like this. The field of a user's
table is of type "smallint", and it turns out that this range is not
sufficient. So change it to "int". At this point, the REPLICA IDENTITY
is lost and the user does not realize it. When they found out, the
logical replication for this period of time did not output normally.
Users have to find other ways to get the data back.
The logical replication of this user is to issue standard SQL statements
to other relational databases using the plugin developed by himself. And
they have thousands of tables to replicate.
So I think this patch is appropriate in this scenario. As for the
matching problem between publishers and subscribers, I'm afraid it's
hard to solve here. If this is not a suitable modification, I can
withdraw it. And see if there's a better way.
If necessary, I'll modify it again. Rebase to the master branch.
Show quoted text
regards.
Re: Restore replication settings when modifying a field type
Em seg, 28 de out de 2019 às 01:41, Kyotaro Horiguchi
<horikyota.ntt@gmail.com> escreveu:
At Sat, 26 Oct 2019 16:50:48 +0800, Quan Zongliang <quanzongliang@gmail.com> wrote in
In fact, the replication property of the table has not been modified,
and it is still 'i'(REPLICA_IDENTITY_INDEX). But the previously
specified index property 'indisreplident' is set to false because of
the rebuild.I suppose that the behavior is intended. Change of column types on the
publisher side can break the agreement on replica identity with
subscribers. Thus replica identity setting cannot be restored
unconditionally. For (somewhat artifitial :p) example:
I don't think so. The actual logical replication behavior is that DDL
will always break replication. If you add a new column or drop a
column, you will stop replication for that table while you don't
execute the same DDL in the subscriber. What happens in the OP case is
that a DDL is *silently* breaking the logical replication. IMHO it is
a bug. If the behavior was intended it should clean
pg_class.relreplident but it is not.
Explicit setting of replica identity premises that they are sure that
the setting works correctly. Implicit rebuilding after a type change
can silently break it.
The current behavior forces the OP to execute 2 DDLs in the same
transaction to ensure that it won't "loose" transactions for logical
replication.
At least we need to guarantee that the restored replica identity
setting is truly compatible with all existing subscribers. I'm not
sure about potential subscribers..
Why? Replication will break and to fix it you should apply the same
DDL you apply in publisher. It is the right thing to do.
[poking the code...]
ATExecAlterColumnType records everything that depends on the column
and for indexes it saves the definition (via pg_get_indexdef_string).
Definition is not sufficient for reconstructing the replica identity
information because there is not such keyword for replica identity in
CREATE INDEX. The new index should call relation_mark_replica_identity
to fix pg_index.indisreplident.
--
Euler Taveira Timbira -
http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Re: Restore replication settings when modifying a field type
On 2019/10/28 12:39, Kyotaro Horiguchi wrote:
Hello.
# The patch no longer applies on the current master. Needs a rebasing.
New patch, rebased on master branch.
Show quoted text
At Sat, 26 Oct 2019 16:50:48 +0800, Quan Zongliang <quanzongliang@gmail.com> wrote in
In fact, the replication property of the table has not been modified,
and it is still 'i'(REPLICA_IDENTITY_INDEX). But the previously
specified index property 'indisreplident' is set to false because of
the rebuild.I suppose that the behavior is intended. Change of column types on the
publisher side can break the agreement on replica identity with
subscribers. Thus replica identity setting cannot be restored
unconditionally. For (somewhat artifitial :p) example:P=# create table t (c1 integer, c2 text unique not null);
P=# alter table t replica identity using index t_c2_key;
P=# create publication p1 for table t;
P=# insert into t values (0, '00'), (1, '01');
S=# create table t (c1 integer, c2 text unique not null);
S=# alter table t replica identity using index t_c2_key;
S=# create subscription s1 connection '...' publication p1;Your patch allows change of the type of c2 into integer.
P=# alter table t alter column c2 type integer using c2::integer;
P=# update t set c1 = c1 + 1 where c2 = '01';This change doesn't affect perhaps as expected.
S=# select * from t;
c1 | c2
----+----
0 | 00
1 | 01
(2 rows)So I developed a patch. If the user modifies the field type. The
associated index is REPLICA IDENTITY. Rebuild and restore replication
settings.Explicit setting of replica identity premises that they are sure that
the setting works correctly. Implicit rebuilding after a type change
can silently break it.At least we need to guarantee that the restored replica identity
setting is truly compatible with all existing subscribers. I'm not
sure about potential subscribers..Anyway I think it is a problem that replica identity setting is
dropped silently. Perhaps a message something like "REPLICA IDENTITY
setting is lost, please redefine after confirmation of compatibility
with subscribers." is needed.regards.
Attachments:
replidfieldtype.patchtext/plain; charset=UTF-8; name=replidfieldtype.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 05593f3316..02f8dbeabf 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -174,6 +174,8 @@ typedef struct AlteredTableInfo
List *changedConstraintDefs; /* string definitions of same */
List *changedIndexOids; /* OIDs of indexes to rebuild */
List *changedIndexDefs; /* string definitions of same */
+ Oid changedReplIdentOid; /* OID of index to reset REPLICA IDENTIFY */
+ char *changedReplIdentDef; /* string definitions of same */
} AlteredTableInfo;
/* Struct describing one new constraint to check in Phase 3 scan */
@@ -459,6 +461,7 @@ static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
AlterTableCmd *cmd, LOCKMODE lockmode);
static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab);
static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab);
+static void RememberReplicaIdentForRebuilding(Oid indoid, AlteredTableInfo *tab);
static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab,
LOCKMODE lockmode);
static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId,
@@ -10715,6 +10718,9 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
{
Assert(foundObject.objectSubId == 0);
RememberIndexForRebuilding(foundObject.objectId, tab);
+
+ if (RelationGetForm(rel)->relreplident==REPLICA_IDENTITY_INDEX)
+ RememberReplicaIdentForRebuilding(foundObject.objectId, tab);
}
else if (relKind == RELKIND_SEQUENCE)
{
@@ -10749,9 +10755,18 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
}
case OCLASS_CONSTRAINT:
- Assert(foundObject.objectSubId == 0);
- RememberConstraintForRebuilding(foundObject.objectId, tab);
- break;
+ {
+ Oid indId;
+
+ Assert(foundObject.objectSubId == 0);
+ RememberConstraintForRebuilding(foundObject.objectId, tab);
+
+ indId = get_constraint_index(foundObject.objectId);
+ if (OidIsValid(indId))
+ RememberReplicaIdentForRebuilding(indId, tab);
+
+ break;
+ }
case OCLASS_REWRITE:
/* XXX someday see if we can cope with revising views */
@@ -11075,6 +11090,36 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab)
}
}
+/*
+ * Subroutine for ATExecAlterColumnType: remember that a replica identify
+ * needs to be reset (which we might already know).
+ */
+static void
+RememberReplicaIdentForRebuilding(Oid indoid, AlteredTableInfo *tab)
+{
+ char *defstring;
+
+ /*
+ * This de-duplication check is critical for two independent reasons: we
+ * mustn't try to recreate the same constraint twice, and if a constraint
+ * depends on more than one column whose type is to be altered, we must
+ * capture its definition string before applying any of the column type
+ * changes. ruleutils.c will get confused if we ask again later.
+ */
+ if (OidIsValid(tab->changedReplIdentOid))
+ return;
+
+ /* OK, capture the constraint's existing definition string */
+ defstring = pg_get_replidentdef_command(indoid);
+
+ /* not a replica identify */
+ if (defstring==NULL)
+ return;
+
+ tab->changedReplIdentOid = indoid;
+ tab->changedReplIdentDef = defstring;
+}
+
/*
* Subroutine for ATExecAlterColumnType: remember that an index needs
* to be rebuilt (which we might already know).
@@ -11222,6 +11267,15 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
add_exact_object_address(&obj, objects);
}
+ if (OidIsValid(tab->changedReplIdentOid))
+ {
+ Oid relid = IndexGetRelation(tab->changedReplIdentOid, false);
+ ATPostAlterTypeParse(InvalidOid, relid, InvalidOid,
+ tab->changedReplIdentDef,
+ wqueue, lockmode, tab->rewrite);
+ }
+
+
/*
* It should be okay to use DROP_RESTRICT here, since nothing else should
* be depending on these objects.
@@ -11373,6 +11427,11 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
NIL,
con->conname);
}
+ else if (cmd->subtype == AT_ReplicaIdentity)
+ {
+ tab->subcmds[AT_PASS_MISC] =
+ lappend(tab->subcmds[AT_PASS_MISC], cmd);
+ }
else if (cmd->subtype == AT_SetNotNull)
{
/*
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3e64390d81..c5efe93c0c 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1854,6 +1854,44 @@ pg_get_partconstrdef_string(Oid partitionId, char *aliasname)
return deparse_expression((Node *) constr_expr, context, true, false);
}
+/*
+ * pg_get_replidentdef_command
+ *
+ * Returns the definition for the REPLICA IDENTITY, ie, everything that needs to
+ * appear after "ALTER TABLE ... REPLICA IDENTITY USING INDEX <indexname>".
+ */
+char *pg_get_replidentdef_command(Oid indId)
+{
+ Oid relId;
+ HeapTuple ht_idx;
+ Form_pg_index idxrec;
+
+ StringInfoData buf;
+
+ ht_idx = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indId));
+ if (!HeapTupleIsValid(ht_idx))
+ elog(ERROR, "cache lookup failed for index %u", indId);
+
+ idxrec = (Form_pg_index) GETSTRUCT(ht_idx);
+ /* not a replica identify */
+ if (!idxrec->indisreplident)
+ {
+ ReleaseSysCache(ht_idx);
+ return NULL;
+ }
+ relId = idxrec->indrelid;
+ ReleaseSysCache(ht_idx);
+
+ initStringInfo(&buf);
+ appendStringInfoString(&buf, "ALTER TABLE ");
+
+ appendStringInfoString(&buf, generate_qualified_relation_name(relId));
+ appendStringInfoString(&buf, " REPLICA IDENTITY USING INDEX ");
+ appendStringInfoString(&buf, quote_identifier(get_rel_name(indId)));
+
+ return buf.data;
+}
+
/*
* pg_get_constraintdef
*
diff --git a/src/include/utils/ruleutils.h b/src/include/utils/ruleutils.h
index d34cad2f4b..71c3da1316 100644
--- a/src/include/utils/ruleutils.h
+++ b/src/include/utils/ruleutils.h
@@ -24,6 +24,8 @@ extern char *pg_get_indexdef_columns(Oid indexrelid, bool pretty);
extern char *pg_get_partkeydef_columns(Oid relid, bool pretty);
extern char *pg_get_partconstrdef_string(Oid partitionId, char *aliasname);
+extern char *pg_get_replidentdef_command(Oid indId);
+
extern char *pg_get_constraintdef_command(Oid constraintId);
extern char *deparse_expression(Node *expr, List *dpcontext,
bool forceprefix, bool showimplicit);
Re: Restore replication settings when modifying a field type
On 2019-11-01 04:39, Euler Taveira wrote:
ATExecAlterColumnType records everything that depends on the column
and for indexes it saves the definition (via pg_get_indexdef_string).
Definition is not sufficient for reconstructing the replica identity
information because there is not such keyword for replica identity in
CREATE INDEX. The new index should call relation_mark_replica_identity
to fix pg_index.indisreplident.
Yeah, I don't think we need to do the full dance of reverse compiling
the SQL command and reexecuting it, as the patch currently does. That's
only necessary for rebuilding the index itself. For re-setting the
replica identity, we can just use the internal API as you say.
Also, a few test cases would be nice for this patch.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Restore replication settings when modifying a field type
On 2020/1/3 17:14, Peter Eisentraut wrote:
On 2019-11-01 04:39, Euler Taveira wrote:
ATExecAlterColumnType records everything that depends on the column
and for indexes it saves the definition (via pg_get_indexdef_string).
Definition is not sufficient for reconstructing the replica identity
information because there is not such keyword for replica identity in
CREATE INDEX. The new index should call relation_mark_replica_identity
to fix pg_index.indisreplident.Yeah, I don't think we need to do the full dance of reverse compiling
the SQL command and reexecuting it, as the patch currently does. That's
only necessary for rebuilding the index itself. For re-setting the
replica identity, we can just use the internal API as you say.Also, a few test cases would be nice for this patch.
I'm a little busy. I'll write a new patch in a few days.
Re: Restore replication settings when modifying a field type
On 2020/1/15 08:30, Quan Zongliang wrote:
On 2020/1/3 17:14, Peter Eisentraut wrote:
On 2019-11-01 04:39, Euler Taveira wrote:
ATExecAlterColumnType records everything that depends on the column
and for indexes it saves the definition (via pg_get_indexdef_string).
Definition is not sufficient for reconstructing the replica identity
information because there is not such keyword for replica identity in
CREATE INDEX. The new index should call relation_mark_replica_identity
to fix pg_index.indisreplident.Yeah, I don't think we need to do the full dance of reverse compiling
the SQL command and reexecuting it, as the patch currently does.
That's only necessary for rebuilding the index itself. For re-setting
the replica identity, we can just use the internal API as you say.Also, a few test cases would be nice for this patch.
I'm a little busy. I'll write a new patch in a few days.
new patch attached.
test case 1:
create table t1 (col1 int,col2 int not null,
col3 int not null,unique(col2,col3));
alter table t1 replica identity using index t1_col2_col3_key;
alter table t1 alter col3 type text;
alter table t1 alter col3 type smallint using col3::int;
test case 2:
create table t2 (col1 varchar(10), col2 text not null,
col3 timestamp not null,unique(col2,col3),
col4 int not null unique);
alter table t2 replica identity using index t2_col2_col3_key;
alter table t2 alter col3 type text;
alter table t2 replica identity using index t2_col4_key;
alter table t2 alter col4 type timestamp using '2020-02-11'::timestamp;
Attachments:
replfieldtype.patchtext/plain; charset=UTF-8; name=replfieldtype.patch; x-mac-creator=0; x-mac-type=0Download
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b7c8d663fc..de4da64e06 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 */
+ List *changedReplIdentName; /* fullname of index to reset REPLICA IDENTIFY */
} AlteredTableInfo;
/* Struct describing one new constraint to check in Phase 3 scan */
@@ -482,6 +483,7 @@ static ObjectAddress ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
AlterTableCmd *cmd, LOCKMODE lockmode);
static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab);
static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab);
+static void RememberReplicaIdentForRebuilding(Oid indoid, AlteredTableInfo *tab);
static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab,
LOCKMODE lockmode);
static void ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId,
@@ -553,6 +555,7 @@ static void refuseDupeIndexAttach(Relation parentIdx, Relation partIdx,
Relation partitionTbl);
static List *GetParentedForeignKeyRefs(Relation partition);
static void ATDetachCheckNoForeignKeyRefs(Relation partition);
+static void relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid, bool is_internal);
/* ----------------------------------------------------------------
@@ -4274,7 +4277,10 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
Relation rel;
ListCell *lcmd;
- if (subcmds == NIL)
+ if (subcmds == NIL && pass != AT_PASS_MISC)
+ continue;
+ if (subcmds == NIL && pass == AT_PASS_MISC &&
+ tab->changedReplIdentName==NIL)
continue;
/*
@@ -4295,6 +4301,18 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode,
if (pass == AT_PASS_ALTER_TYPE)
ATPostAlterTypeCleanup(wqueue, tab, lockmode);
+ if (pass == AT_PASS_MISC && tab->changedReplIdentName)
+ {
+ Relation idxrel;
+ ObjectAddress address = get_object_address(OBJECT_INDEX,
+ (Node*)tab->changedReplIdentName, &idxrel,
+ AccessExclusiveLock, false);
+ relation_close(idxrel, NoLock);
+
+ relation_mark_replica_identity(rel, REPLICA_IDENTITY_INDEX,
+ address.objectId, true);
+ }
+
relation_close(rel, NoLock);
}
}
@@ -11231,6 +11249,9 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
{
Assert(foundObject.objectSubId == 0);
RememberIndexForRebuilding(foundObject.objectId, tab);
+ if (RelationGetForm(rel)->relreplident==REPLICA_IDENTITY_INDEX &&
+ index_is_replident(foundObject.objectId))
+ RememberReplicaIdentForRebuilding(foundObject.objectId, tab);
}
else if (relKind == RELKIND_SEQUENCE)
{
@@ -11265,9 +11286,20 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
}
case OCLASS_CONSTRAINT:
- Assert(foundObject.objectSubId == 0);
- RememberConstraintForRebuilding(foundObject.objectId, tab);
- break;
+ {
+ Oid indId;
+
+ Assert(foundObject.objectSubId == 0);
+ RememberConstraintForRebuilding(foundObject.objectId, tab);
+
+ indId = get_constraint_index(foundObject.objectId);
+ if (OidIsValid(indId) &&
+ RelationGetForm(rel)->relreplident==REPLICA_IDENTITY_INDEX &&
+ index_is_replident(indId))
+ RememberReplicaIdentForRebuilding(indId, tab);
+
+ break;
+ }
case OCLASS_REWRITE:
/* XXX someday see if we can cope with revising views */
@@ -11590,6 +11622,26 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab)
}
}
+/*
+ * Subroutine for ATExecAlterColumnType: remember that a replica identify
+ * needs to be reset (which we might already know).
+ */
+static void
+RememberReplicaIdentForRebuilding(Oid indoid, AlteredTableInfo *tab)
+{
+ char *nspName;
+ char *relName;
+
+ /* just in case */
+ if (tab->changedReplIdentName)
+ return;
+
+ relName = get_rel_name(indoid);
+ nspName = get_namespace_name(get_rel_namespace(indoid));
+
+ tab->changedReplIdentName = list_make2(makeString(nspName), makeString(relName));
+}
+
/*
* Subroutine for ATExecAlterColumnType: remember that an index needs
* to be rebuilt (which we might already know).
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index fb0599f7f1..d74c5a1427 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3227,3 +3227,21 @@ get_index_column_opclass(Oid index_oid, int attno)
return opclass;
}
+
+bool
+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 370f62a133..b91cb84b15 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 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 */
Re: Restore replication settings when modifying a field type
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.patchtext/plain; charset=UTF-8; name=v4-0001-Preserve-replica-identity-index-across-ALTER-TABL.patch; x-mac-creator=0; x-mac-type=0Download
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.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
Re: Restore replication settings when modifying a field type
On Thu, 5 Mar 2020 at 09:45, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
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.LGTM. Tests are ok. I've rebased it (because
61d7c7bce3686ec02bd64abac742dd35ed9b9b01). Are you planning to backpatch
it? IMHO you should because it is a bug (since REPLICA IDENTITY was
introduced in 9.4). This patch can be applied as-is in 12 but not to other
older branches. I attached new patches.
Regards,
--
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v5-11-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchapplication/x-patch; name=v5-11-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchDownload
From e35463df17f25940eabe92142237c2a85fb15b6f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 5 Mar 2020 10:11:13 +0100
Subject: [PATCH] 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 38386fb9cf..80725dcf92 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -175,6 +175,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 */
@@ -10314,6 +10315,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).
@@ -10332,11 +10349,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);
}
}
@@ -10379,6 +10401,8 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab)
indoid);
tab->changedIndexDefs = lappend(tab->changedIndexDefs,
defstring);
+
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
}
}
@@ -10593,6 +10617,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 a4da935371..4a2510e2ed 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3156,3 +3156,26 @@ get_range_collation(Oid rangeOid)
else
return InvalidOid;
}
+
+/*
+ * 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 c97c926992..b17931e297 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -178,6 +178,7 @@ extern char *get_namespace_name(Oid nspid);
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_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 67c34a92a4..65c7c9ac9f 100644
--- a/src/test/regress/expected/replica_identity.out
+++ b/src/test/regress/expected/replica_identity.out
@@ -186,5 +186,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 3d2171c733..5277ff39cd 100644
--- a/src/test/regress/sql/replica_identity.sql
+++ b/src/test/regress/sql/replica_identity.sql
@@ -79,5 +79,26 @@ SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
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.20.1
v5-12-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchapplication/x-patch; name=v5-12-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchDownload
From 8a032fc0a5aaf28593b4aee583431cc1409e5773 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 5 Mar 2020 10:11:13 +0100
Subject: [PATCH] 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 d07476d8dd..ce24955d55 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -174,6 +174,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 */
@@ -11138,6 +11139,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).
@@ -11156,11 +11173,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);
}
}
@@ -11203,6 +11225,8 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab)
indoid);
tab->changedIndexDefs = lappend(tab->changedIndexDefs,
defstring);
+
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
}
}
@@ -11311,6 +11335,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 167d2592d8..c351a59e53 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3250,3 +3250,26 @@ get_index_isvalid(Oid index_oid)
return isvalid;
}
+
+/*
+ * 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 810dbeeab9..f853724431 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -182,6 +182,7 @@ 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_isvalid(Oid index_oid);
+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 175ecd2879..a586347eee 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 @@ SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
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.20.1
v5-96-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchapplication/x-patch; name=v5-96-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchDownload
From e454c38afe989cd89608cc838bc6bfb804e46b29 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 5 Mar 2020 10:11:13 +0100
Subject: [PATCH] Preserve replica identity index across ALTER TABLE rewrite
---
src/backend/commands/tablecmds.c | 46 +++++++++++++++++++
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, 137 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 9c6ca7ff9c..380c52e8df 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -168,6 +168,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 */
@@ -8596,6 +8597,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).
@@ -8615,6 +8632,7 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab,
{
/* OK, capture the constraint's existing definition string */
char *defstring = pg_get_constraintdef_command(conoid);
+ Oid indoid;
/*
* Put NORMAL dependencies at the front of the list and AUTO
@@ -8630,6 +8648,10 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab,
tab->changedConstraintOids);
tab->changedConstraintDefs = lcons(defstring,
tab->changedConstraintDefs);
+
+ indoid = get_constraint_index(conoid);
+ if (OidIsValid(indoid))
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
else
{
@@ -8637,6 +8659,10 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab,
conoid);
tab->changedConstraintDefs = lappend(tab->changedConstraintDefs,
defstring);
+
+ indoid = get_constraint_index(conoid);
+ if (OidIsValid(indoid))
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
}
}
@@ -8681,6 +8707,8 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab)
indoid);
tab->changedIndexDefs = lappend(tab->changedIndexDefs,
defstring);
+
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
}
}
@@ -8874,6 +8902,24 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
wqueue, lockmode, tab->rewrite);
}
+ /*
+ * 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);
+ }
+
/*
* 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 e39c1d45cd..c49efd5e54 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3088,3 +3088,26 @@ get_range_collation(Oid rangeOid)
else
return InvalidOid;
}
+
+/*
+ * 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 b4757a3466..54a4dc8200 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -160,6 +160,7 @@ extern char *get_namespace_name(Oid nspid);
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_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 27e2b67b8d..1071af9247 100644
--- a/src/test/regress/expected/replica_identity.out
+++ b/src/test/regress/expected/replica_identity.out
@@ -187,5 +187,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 | Modifiers
+--------+---------+-----------
+ 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 | Modifiers
+--------+--------+-----------
+ 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 | Modifiers
+--------+---------+-----------
+ 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 | Modifiers
+--------+--------+-----------
+ 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 3d2171c733..5277ff39cd 100644
--- a/src/test/regress/sql/replica_identity.sql
+++ b/src/test/regress/sql/replica_identity.sql
@@ -79,5 +79,26 @@ SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
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.20.1
v5-95-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchapplication/x-patch; name=v5-95-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchDownload
From 0463753e67d959a22e919d5173206869c1fa2ebd Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 5 Mar 2020 10:11:13 +0100
Subject: [PATCH] Preserve replica identity index across ALTER TABLE rewrite
---
src/backend/commands/tablecmds.c | 46 +++++++++++++++++++
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, 137 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7fcc3db14c..244e237d57 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -166,6 +166,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 */
@@ -8566,6 +8567,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).
@@ -8585,6 +8602,7 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab,
{
/* OK, capture the constraint's existing definition string */
char *defstring = pg_get_constraintdef_command(conoid);
+ Oid indoid;
/*
* Put NORMAL dependencies at the front of the list and AUTO
@@ -8600,6 +8618,10 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab,
tab->changedConstraintOids);
tab->changedConstraintDefs = lcons(defstring,
tab->changedConstraintDefs);
+
+ indoid = get_constraint_index(conoid);
+ if (OidIsValid(indoid))
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
else
{
@@ -8607,6 +8629,10 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab,
conoid);
tab->changedConstraintDefs = lappend(tab->changedConstraintDefs,
defstring);
+
+ indoid = get_constraint_index(conoid);
+ if (OidIsValid(indoid))
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
}
}
@@ -8651,6 +8677,8 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab)
indoid);
tab->changedIndexDefs = lappend(tab->changedIndexDefs,
defstring);
+
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
}
}
@@ -8844,6 +8872,24 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
wqueue, lockmode, tab->rewrite);
}
+ /*
+ * 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);
+ }
+
/*
* 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 b0b024dc3e..5a07ce4118 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3045,3 +3045,26 @@ get_range_collation(Oid rangeOid)
else
return InvalidOid;
}
+
+/*
+ * 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 489b196ecf..b8f5a9bf00 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -158,6 +158,7 @@ extern char *get_namespace_name(Oid nspid);
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_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 0da54a0544..332f301413 100644
--- a/src/test/regress/expected/replica_identity.out
+++ b/src/test/regress/expected/replica_identity.out
@@ -187,5 +187,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 | Modifiers
+--------+---------+-----------
+ 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 | Modifiers
+--------+--------+-----------
+ 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 | Modifiers
+--------+---------+-----------
+ 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 | Modifiers
+--------+--------+-----------
+ 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 bb71fdac96..ab4c72b23a 100644
--- a/src/test/regress/sql/replica_identity.sql
+++ b/src/test/regress/sql/replica_identity.sql
@@ -79,5 +79,26 @@ SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
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.20.1
v5-10-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchapplication/x-patch; name=v5-10-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchDownload
From 3760d0f2060249e750200daadabbbe557cd105a1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 5 Mar 2020 10:11:13 +0100
Subject: [PATCH] Preserve replica identity index across ALTER TABLE rewrite
---
src/backend/commands/tablecmds.c | 46 +++++++++++++++++++
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, 137 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1a005760d8..25011f671e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -173,6 +173,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 */
@@ -9420,6 +9421,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).
@@ -9439,6 +9456,7 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab,
{
/* OK, capture the constraint's existing definition string */
char *defstring = pg_get_constraintdef_command(conoid);
+ Oid indoid;
/*
* Put NORMAL dependencies at the front of the list and AUTO
@@ -9454,6 +9472,10 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab,
tab->changedConstraintOids);
tab->changedConstraintDefs = lcons(defstring,
tab->changedConstraintDefs);
+
+ indoid = get_constraint_index(conoid);
+ if (OidIsValid(indoid))
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
else
{
@@ -9461,6 +9483,10 @@ RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab,
conoid);
tab->changedConstraintDefs = lappend(tab->changedConstraintDefs,
defstring);
+
+ indoid = get_constraint_index(conoid);
+ if (OidIsValid(indoid))
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
}
}
@@ -9505,6 +9531,8 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab)
indoid);
tab->changedIndexDefs = lappend(tab->changedIndexDefs,
defstring);
+
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
}
}
@@ -9697,6 +9725,24 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
wqueue, lockmode, tab->rewrite);
}
+ /*
+ * 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);
+ }
+
/*
* 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 88415e6892..dd7e606c16 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3138,3 +3138,26 @@ get_range_collation(Oid rangeOid)
else
return InvalidOid;
}
+
+/*
+ * 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 e9ef5cba34..dbc28ffd2a 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -178,6 +178,7 @@ extern char *get_namespace_name(Oid nspid);
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_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 67c34a92a4..65c7c9ac9f 100644
--- a/src/test/regress/expected/replica_identity.out
+++ b/src/test/regress/expected/replica_identity.out
@@ -186,5 +186,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 3d2171c733..5277ff39cd 100644
--- a/src/test/regress/sql/replica_identity.sql
+++ b/src/test/regress/sql/replica_identity.sql
@@ -79,5 +79,26 @@ SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
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.20.1
v5-master-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchapplication/x-patch; name=v5-master-0001-Preserve-replica-identity-index-across-ALTER-TABLE-r.patchDownload
From 11a3201c02afae1c8a4550610afdc1d88c6cca6b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 5 Mar 2020 10:11:13 +0100
Subject: [PATCH] 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 3eb861bfbf..347869d468 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 */
@@ -11562,6 +11563,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).
@@ -11580,11 +11597,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);
}
}
@@ -11627,6 +11649,8 @@ RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab)
indoid);
tab->changedIndexDefs = lappend(tab->changedIndexDefs,
defstring);
+
+ RememberReplicaIdentityForRebuilding(indoid, tab);
}
}
}
@@ -11735,6 +11759,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 400e7689fe..4fb3a08183 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -3250,3 +3250,26 @@ get_index_isvalid(Oid index_oid)
return isvalid;
}
+
+/*
+ * 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 131d10eab0..7290b3135f 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -182,6 +182,7 @@ 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_isvalid(Oid index_oid);
+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 @@ SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
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.20.1
Re: Restore replication settings when modifying a field type
On 2020-03-10 14:16, Euler Taveira wrote:
On Thu, 5 Mar 2020 at 09:45, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>> wrote: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.LGTM. Tests are ok. I've rebased it (because
61d7c7bce3686ec02bd64abac742dd35ed9b9b01). Are you planning to backpatch
it? IMHO you should because it is a bug (since REPLICA IDENTITY was
introduced in 9.4). This patch can be applied as-is in 12 but not to
other older branches. I attached new patches.
Thanks. This has been committed and backpatched to 9.5.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services