From 6f5276cef8814300057a370778bb9b8ce6ebde23 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Mon, 18 Dec 2023 16:39:30 +0530 Subject: [PATCH 08/27] DROP IDENTITY on partitioned table recurses to its partitions Since identity property is inherited by partition tables, dropping identity of a column in partitioned table should result in dropping it from all the partitions. I tried to implement the DROP SEQUENCE as a separate step to be executed at the end of ALTER TABLE command. But that does not work since the sequence has a dependency on the column (whose identity is being dropped) and not on the identity property itself. There is no step/ALTER TABLE command to drop that dependecy. Dropping identity property from a column of a partition table is not allwoed. Ashutosh Bapat --- src/backend/commands/tablecmds.c | 73 +++++++++++++++++++++----- src/test/regress/expected/identity.out | 26 +++++++++ src/test/regress/sql/identity.sql | 10 ++++ 3 files changed, 96 insertions(+), 13 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 5ae0739377..f0c1a85031 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -456,7 +456,9 @@ static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName, bool recurse, bool recursing); static ObjectAddress ATExecSetIdentity(Relation rel, const char *colName, Node *def, LOCKMODE lockmode); -static ObjectAddress ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode); +static ObjectAddress ATExecDropIdentity(Relation rel, const char *colName, + bool missing_ok, LOCKMODE lockmode, + bool recurse, bool recursing); static void ATPrepDropExpression(Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing, LOCKMODE lockmode); static ObjectAddress ATExecDropExpression(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode); static ObjectAddress ATExecSetStatistics(Relation rel, const char *colName, int16 colNum, @@ -4839,7 +4841,9 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, break; case AT_DropIdentity: ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_VIEW | ATT_FOREIGN_TABLE); - /* This command never recurses */ + /* Set up recursion for phase 2; no other prep needed */ + if (recurse) + cmd->recurse = true; pass = AT_PASS_DROP; break; case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */ @@ -5237,7 +5241,8 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, address = ATExecSetIdentity(rel, cmd->name, cmd->def, lockmode); break; case AT_DropIdentity: - address = ATExecDropIdentity(rel, cmd->name, cmd->missing_ok, lockmode); + address = ATExecDropIdentity(rel, cmd->name, cmd->missing_ok, + lockmode, cmd->recurse, false); break; case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */ address = ATExecDropNotNull(rel, cmd->name, cmd->recurse, lockmode); @@ -8297,7 +8302,9 @@ ATExecSetIdentity(Relation rel, const char *colName, Node *def, LOCKMODE lockmod * Return the address of the affected column. */ static ObjectAddress -ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE lockmode) +ATExecDropIdentity(Relation rel, const char *colName, + bool missing_ok, LOCKMODE lockmode, + bool recurse, bool recursing) { HeapTuple tuple; Form_pg_attribute attTup; @@ -8306,6 +8313,19 @@ ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE ObjectAddress address; Oid seqid; ObjectAddress seqaddress; + bool ispartitioned; + + ispartitioned = (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); + if (ispartitioned && !recurse) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot drop identity from a column of only the partitioned table"), + errhint("Do not specify the ONLY keyword."))); + + if (rel->rd_rel->relispartition && !recursing) + ereport(ERROR, + errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("cannot drop identity from a column of a partition")); attrelation = table_open(AttributeRelationId, RowExclusiveLock); tuple = SearchSysCacheCopyAttName(RelationGetRelid(rel), colName); @@ -8354,15 +8374,42 @@ ATExecDropIdentity(Relation rel, const char *colName, bool missing_ok, LOCKMODE table_close(attrelation, RowExclusiveLock); - /* drop the internal sequence */ - seqid = getIdentitySequence(RelationGetRelid(rel), attnum, false); - deleteDependencyRecordsForClass(RelationRelationId, seqid, - RelationRelationId, DEPENDENCY_INTERNAL); - CommandCounterIncrement(); - seqaddress.classId = RelationRelationId; - seqaddress.objectId = seqid; - seqaddress.objectSubId = 0; - performDeletion(&seqaddress, DROP_RESTRICT, PERFORM_DELETION_INTERNAL); + /* + * Recurse to drop the identity from column in partitions. Identity is not + * inherited in regular inheritance children so ignore them. + */ + if (recurse && ispartitioned) + { + List *children; + ListCell *lc; + + children = find_inheritance_children(RelationGetRelid(rel), + lockmode); + + foreach(lc, children) + { + Relation childrel; + + childrel = table_open(lfirst_oid(lc), NoLock); + ATExecDropIdentity(childrel, colName, false, lockmode, recurse, + true); + table_close(childrel, NoLock); + } + } + + if (!recursing) + { + /* drop the internal sequence */ + seqid = getIdentitySequence(RelationGetRelid(rel), attnum, false); + deleteDependencyRecordsForClass(RelationRelationId, seqid, + RelationRelationId, + DEPENDENCY_INTERNAL); + CommandCounterIncrement(); + seqaddress.classId = RelationRelationId; + seqaddress.objectId = seqid; + seqaddress.objectSubId = 0; + performDeletion(&seqaddress, DROP_RESTRICT, PERFORM_DELETION_INTERNAL); + } return address; } diff --git a/src/test/regress/expected/identity.out b/src/test/regress/expected/identity.out index 9de35470af..d2860b4347 100644 --- a/src/test/regress/expected/identity.out +++ b/src/test/regress/expected/identity.out @@ -617,6 +617,32 @@ SELECT tableoid::regclass, f1, f2, f3 FROM pitest3; pitest3_p1 | 07-05-2016 | from pitest3_p1 | 4 (4 rows) +-- changing an identity column to a non-identity column in a partitioned table +ALTER TABLE pitest3_p1 ALTER COLUMN f3 DROP IDENTITY; -- fails +ERROR: cannot drop identity from a column of a partition +ALTER TABLE ONLY pitest3 ALTER COLUMN f3 DROP IDENTITY; -- fails +ERROR: cannot drop identity from a column of only the partitioned table +HINT: Do not specify the ONLY keyword. +ALTER TABLE pitest3 ALTER COLUMN f3 DROP IDENTITY; +INSERT into pitest3(f1, f2) VALUES ('2016-07-4', 'from pitest3'); -- fails +ERROR: null value in column "f3" of relation "pitest3_p1" violates not-null constraint +DETAIL: Failing row contains (07-04-2016, from pitest3, null). +INSERT into pitest3_p1 (f1, f2) VALUES ('2016-07-5', 'from pitest3_p1'); -- fails +ERROR: null value in column "f3" of relation "pitest3_p1" violates not-null constraint +DETAIL: Failing row contains (07-05-2016, from pitest3_p1, null). +INSERT into pitest3(f1, f2, f3) VALUES ('2016-07-6', 'from pitest3', 5); +INSERT into pitest3_p1 (f1, f2, f3) VALUES ('2016-07-7', 'from pitest3_p1', 6); +SELECT tableoid::regclass, f1, f2, f3 FROM pitest3; + tableoid | f1 | f2 | f3 +------------+------------+-----------------+---- + pitest3_p1 | 07-02-2016 | from pitest3 | 1 + pitest3_p1 | 07-03-2016 | from pitest3_p1 | 2 + pitest3_p1 | 07-04-2016 | from pitest3 | 3 + pitest3_p1 | 07-05-2016 | from pitest3_p1 | 4 + pitest3_p1 | 07-06-2016 | from pitest3 | 5 + pitest3_p1 | 07-07-2016 | from pitest3_p1 | 6 +(6 rows) + -- partition with identity column of its own is not allowed CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1); CREATE TABLE itest_child PARTITION OF itest_parent ( diff --git a/src/test/regress/sql/identity.sql b/src/test/regress/sql/identity.sql index 9d1effa059..62323f9cbd 100644 --- a/src/test/regress/sql/identity.sql +++ b/src/test/regress/sql/identity.sql @@ -382,6 +382,16 @@ INSERT into pitest3(f1, f2) VALUES ('2016-07-4', 'from pitest3'); INSERT into pitest3_p1 (f1, f2) VALUES ('2016-07-5', 'from pitest3_p1'); SELECT tableoid::regclass, f1, f2, f3 FROM pitest3; +-- changing an identity column to a non-identity column in a partitioned table +ALTER TABLE pitest3_p1 ALTER COLUMN f3 DROP IDENTITY; -- fails +ALTER TABLE ONLY pitest3 ALTER COLUMN f3 DROP IDENTITY; -- fails +ALTER TABLE pitest3 ALTER COLUMN f3 DROP IDENTITY; +INSERT into pitest3(f1, f2) VALUES ('2016-07-4', 'from pitest3'); -- fails +INSERT into pitest3_p1 (f1, f2) VALUES ('2016-07-5', 'from pitest3_p1'); -- fails +INSERT into pitest3(f1, f2, f3) VALUES ('2016-07-6', 'from pitest3', 5); +INSERT into pitest3_p1 (f1, f2, f3) VALUES ('2016-07-7', 'from pitest3_p1', 6); +SELECT tableoid::regclass, f1, f2, f3 FROM pitest3; + -- partition with identity column of its own is not allowed CREATE TABLE itest_parent (f1 date NOT NULL, f2 text, f3 bigint) PARTITION BY RANGE (f1); CREATE TABLE itest_child PARTITION OF itest_parent ( -- 2.25.1