From: Quan Zongliang <quanzongliang@gmail.com>
To: Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Restore replication settings when modifying a field type
Date: Sat, 26 Oct 2019 16:50:48 +0800
Message-ID: <c70fcab2-4866-0d9f-1d01-e75e189db342@gmail.com> (raw)
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.diff (text/plain, 6.0K) downloaddiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 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.cindex 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.hindex 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);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: quanzongliang@gmail.com
Subject: Re: Restore replication settings when modifying a field type
In-Reply-To: <c70fcab2-4866-0d9f-1d01-e75e189db342@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox