From: Tom Lane <tgl@sss.pgh.pa.us>
To: Japin Li <japinli@hotmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: holly.roberts@starlingbank.com
Cc: peter@eisentraut.org
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17409: Unable to alter data type of clustered column which is referenced by foreign key
Date: Thu, 10 Mar 2022 19:37:45 -0500
Message-ID: <755474.1646959065@sss.pgh.pa.us> (raw)
In-Reply-To: <MEYP282MB1669405BB9B46FBED34EB309B6379@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
References: <17409-52871dda8b5741cb@postgresql.org>
<MEYP282MB16691B29F4FEA0CC45CD5FFAB6369@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
<ME3P282MB1667E2B286DA22A16FD8A8FFB6369@ME3P282MB1667.AUSP282.PROD.OUTLOOK.COM>
<ME3P282MB1667F5A392BB4F26E6B0D82AB6369@ME3P282MB1667.AUSP282.PROD.OUTLOOK.COM>
<Yg8gGrJebITuPKu8@paquier.xyz>
<MEYP282MB1669405BB9B46FBED34EB309B6379@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
Japin Li <japinli@hotmail.com> writes:
> Attach a new patch to fix the replica identify case, also add test cases.
Now that we realize we need to de-duplicate, it seems to me we should
postpone the get_rel_name() calls so that we don't have to do that
work repeatedly; as attached.
Also, while I've not done anything about it here, the proposed test
cases seem remarkably cavalier about their choices of test table
names. If you want to use names as generic as "parent" and "child",
they'd better be temp tables to avoid risk of conflict against other
concurrent regression tests. But most of alter_table.sql prefers
to use names starting with "at".
regards, tom lane
Attachments:
[text/x-diff] v3-fix-alter-data-type-of-clustered-column.patch (4.9K, ../755474.1646959065@sss.pgh.pa.us/2-v3-fix-alter-data-type-of-clustered-column.patch)
download | inline diff:diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex dc5872f988..19c924b7df 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -189,8 +189,8 @@ 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 */- char *clusterOnIndex; /* index to use for CLUSTER */+ Oid replicaIdentityIndex; /* index to reset as REPLICA IDENTITY */+ Oid clusterOnIndex; /* index to use for CLUSTER */
List *changedStatisticsOids; /* OIDs of statistics to rebuild */
List *changedStatisticsDefs; /* string definitions of same */
} AlteredTableInfo;
@@ -12856,10 +12856,12 @@ RememberReplicaIdentityForRebuilding(Oid indoid, AlteredTableInfo *tab)
if (!get_index_isreplident(indoid))
return;
- if (tab->replicaIdentityIndex)+ /* We must de-duplicate multiple requests */+ if (OidIsValid(tab->replicaIdentityIndex) &&+ tab->replicaIdentityIndex != indoid)
elog(ERROR, "relation %u has multiple indexes marked as replica identity", tab->relid);
- tab->replicaIdentityIndex = get_rel_name(indoid);+ tab->replicaIdentityIndex = indoid;
}
/*
@@ -12871,10 +12873,12 @@ RememberClusterOnForRebuilding(Oid indoid, AlteredTableInfo *tab)
if (!get_index_isclustered(indoid))
return;
- if (tab->clusterOnIndex)+ /* We must de-duplicate multiple requests */+ if (OidIsValid(tab->clusterOnIndex) &&+ tab->clusterOnIndex != indoid)
elog(ERROR, "relation %u has multiple clustered indexes", tab->relid);
- tab->clusterOnIndex = get_rel_name(indoid);+ tab->clusterOnIndex = indoid;
}
/*
@@ -13117,13 +13121,15 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
/*
* Queue up command to restore replica identity index marking
*/
- if (tab->replicaIdentityIndex)+ if (OidIsValid(tab->replicaIdentityIndex))
{
AlterTableCmd *cmd = makeNode(AlterTableCmd);
ReplicaIdentityStmt *subcmd = makeNode(ReplicaIdentityStmt);
subcmd->identity_type = REPLICA_IDENTITY_INDEX;
- subcmd->name = tab->replicaIdentityIndex;+ subcmd->name = get_rel_name(tab->replicaIdentityIndex);+ Assert(subcmd->name);+
cmd->subtype = AT_ReplicaIdentity;
cmd->def = (Node *) subcmd;
@@ -13135,12 +13141,13 @@ ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode)
/*
* Queue up command to restore marking of index used for cluster.
*/
- if (tab->clusterOnIndex)+ if (OidIsValid(tab->clusterOnIndex))
{
AlterTableCmd *cmd = makeNode(AlterTableCmd);
cmd->subtype = AT_ClusterOn;
- cmd->name = tab->clusterOnIndex;+ cmd->name = get_rel_name(tab->clusterOnIndex);+ Assert(cmd->name);
/* do it after indexes and constraints */
tab->subcmds[AT_PASS_OLD_CONSTR] =
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.outindex 16e0475663..53a19e38c9 100644--- a/src/test/regress/expected/alter_table.out+++ b/src/test/regress/expected/alter_table.out@@ -4584,3 +4584,16 @@ drop publication pub1;
drop schema alter1 cascade;
NOTICE: drop cascades to table alter1.t1
drop schema alter2 cascade;
+/* Test case for bug #17409 */+create table parent (p1 int constraint pk_parent primary key);+create table child (c1 int, constraint fk_child foreign key (c1) references parent(p1));+cluster parent using pk_parent;+alter table parent alter column p1 set data type bigint;+drop table child;+drop table parent;+create table parent (p1 int constraint pk_parent primary key);+alter table parent replica identity using index pk_parent;+create table child (c1 int, constraint fk_child foreign key (c1) references parent(p1));+alter table parent alter column p1 set data type bigint;+drop table child;+drop table parent;diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sqlindex ac894c0602..38ed8a7134 100644--- a/src/test/regress/sql/alter_table.sql+++ b/src/test/regress/sql/alter_table.sql@@ -3013,3 +3013,19 @@ alter table alter1.t1 set schema alter2; -- should fail
drop publication pub1;
drop schema alter1 cascade;
drop schema alter2 cascade;
++/* Test case for bug #17409 */++create table parent (p1 int constraint pk_parent primary key);+create table child (c1 int, constraint fk_child foreign key (c1) references parent(p1));+cluster parent using pk_parent;+alter table parent alter column p1 set data type bigint;+drop table child;+drop table parent;++create table parent (p1 int constraint pk_parent primary key);+alter table parent replica identity using index pk_parent;+create table child (c1 int, constraint fk_child foreign key (c1) references parent(p1));+alter table parent alter column p1 set data type bigint;+drop table child;+drop table parent;
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-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, japinli@hotmail.com, michael@paquier.xyz, holly.roberts@starlingbank.com, peter@eisentraut.org, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17409: Unable to alter data type of clustered column which is referenced by foreign key
In-Reply-To: <755474.1646959065@sss.pgh.pa.us>
* 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