aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlvaro Herrera <alvherre@alvh.no-ip.org>2024-09-30 11:58:13 +0200
committerAlvaro Herrera <alvherre@alvh.no-ip.org>2024-09-30 11:58:13 +0200
commit4dea33ce765d65d8807d343ca6535a3d067a63da (patch)
tree1b93c52362d032b785ec662bf234d198221d0c87
parent4c7cd07aa62abc29e6885e95b5307e5e08bb3bf7 (diff)
downloadpostgresql-4dea33ce765d65d8807d343ca6535a3d067a63da.tar.gz
postgresql-4dea33ce765d65d8807d343ca6535a3d067a63da.zip
Don't disallow DROP of constraints ONLY on partitioned tables
This restriction seems to have come about due to some fuzzy thinking: in commit 9139aa19423b we were adding a restriction against ADD constraint ONLY on partitioned tables (which is sensible) and apparently we thought the DROP case had to be symmetrical. However, it isn't, and the comments about it are mistaken about the effect it would have. Remove this limitation. There have been no reports of users bothered by this limitation, so I'm not backpatching it just yet. We can revisit this decision later, as needed. Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/202409261752.nbvlawkxsttf@alvherre.pgsql Discussion: https://postgr.es/m/7682253a-6f79-6a92-00aa-267c4c412870@lab.ntt.co.jp (about commit 9139aa19423b, previously not registered)
-rw-r--r--doc/src/sgml/ref/alter_table.sgml4
-rw-r--r--src/backend/commands/tablecmds.c38
-rw-r--r--src/test/regress/expected/alter_table.out21
-rw-r--r--src/test/regress/sql/alter_table.sql7
4 files changed, 22 insertions, 48 deletions
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 1a49f321cf7..925f1084e00 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -251,7 +251,9 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
table. Even if there is no <literal>NOT NULL</literal> constraint on the
parent, such a constraint can still be added to individual partitions,
if desired; that is, the children can disallow nulls even if the parent
- allows them, but not the other way around.
+ allows them, but not the other way around. It is also possible to drop
+ the <literal>NOT NULL</literal> constraint from <literal>ONLY</literal>
+ the parent table, which does not remove it from the children.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7405023e77b..96373323b8d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -447,7 +447,6 @@ static bool check_for_column_name_collision(Relation rel, const char *colname,
bool if_not_exists);
static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid typid);
static void add_column_collation_dependency(Oid relid, int32 attnum, Oid collid);
-static void ATPrepDropNotNull(Relation rel, bool recurse, bool recursing);
static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode);
static void ATPrepSetNotNull(List **wqueue, Relation rel,
AlterTableCmd *cmd, bool recurse, bool recursing,
@@ -4858,7 +4857,6 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */
ATSimplePermissions(cmd->subtype, rel,
ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE);
- ATPrepDropNotNull(rel, recurse, recursing);
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
pass = AT_PASS_DROP;
break;
@@ -7498,29 +7496,7 @@ add_column_collation_dependency(Oid relid, int32 attnum, Oid collid)
/*
* ALTER TABLE ALTER COLUMN DROP NOT NULL
- */
-
-static void
-ATPrepDropNotNull(Relation rel, bool recurse, bool recursing)
-{
- /*
- * If the parent is a partitioned table, like check constraints, we do not
- * support removing the NOT NULL while partitions exist.
- */
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- {
- PartitionDesc partdesc = RelationGetPartitionDesc(rel, true);
-
- Assert(partdesc != NULL);
- if (partdesc->nparts > 0 && !recurse && !recursing)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("cannot remove constraint from only the partitioned table when partitions exist"),
- errhint("Do not specify the ONLY keyword.")));
- }
-}
-
-/*
+ *
* Return the address of the modified column. If the column was already
* nullable, InvalidObjectAddress is returned.
*/
@@ -12713,18 +12689,6 @@ ATExecDropConstraint(Relation rel, const char *constrName,
else
children = NIL;
- /*
- * For a partitioned table, if partitions exist and we are told not to
- * recurse, it's a user error. It doesn't make sense to have a constraint
- * be defined only on the parent, especially if it's a partitioned table.
- */
- if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
- children != NIL && !recurse)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("cannot remove constraint from only the partitioned table when partitions exist"),
- errhint("Do not specify the ONLY keyword.")));
-
foreach_oid(childrelid, children)
{
Relation childrel;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 3b3b0738d79..fb8db37623d 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4401,7 +4401,7 @@ ALTER TABLE part_2 RENAME COLUMN b to c;
ERROR: cannot rename inherited column "b"
ALTER TABLE part_2 ALTER COLUMN b TYPE text;
ERROR: cannot alter inherited column "b"
--- cannot add/drop NOT NULL or check constraints to *only* the parent, when
+-- cannot add NOT NULL or check constraints to *only* the parent, when
-- partitions exist
ALTER TABLE ONLY list_parted2 ALTER b SET NOT NULL;
ERROR: constraint must be added to child tables too
@@ -4409,20 +4409,27 @@ DETAIL: Column "b" of relation "part_2" is not already NOT NULL.
HINT: Do not specify the ONLY keyword.
ALTER TABLE ONLY list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
ERROR: constraint must be added to child tables too
+-- dropping them is ok though
ALTER TABLE list_parted2 ALTER b SET NOT NULL;
ALTER TABLE ONLY list_parted2 ALTER b DROP NOT NULL;
-ERROR: cannot remove constraint from only the partitioned table when partitions exist
-HINT: Do not specify the ONLY keyword.
ALTER TABLE list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_b;
-ERROR: cannot remove constraint from only the partitioned table when partitions exist
-HINT: Do not specify the ONLY keyword.
+-- ... and the partitions should still have both
+\d+ part_2
+ Table "public.part_2"
+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
+--------+--------------+-----------+----------+---------+----------+--------------+-------------
+ a | integer | | | | plain | |
+ b | character(1) | | not null | | extended | |
+Partition of: list_parted2 FOR VALUES IN (2)
+Partition constraint: ((a IS NOT NULL) AND (a = 2))
+Check constraints:
+ "check_b" CHECK (b <> 'zz'::bpchar)
+
-- It's alright though, if no partitions are yet created
CREATE TABLE parted_no_parts (a int) PARTITION BY LIST (a);
ALTER TABLE ONLY parted_no_parts ALTER a SET NOT NULL;
ALTER TABLE ONLY parted_no_parts ADD CONSTRAINT check_a CHECK (a > 0);
-ALTER TABLE ONLY parted_no_parts ALTER a DROP NOT NULL;
-ALTER TABLE ONLY parted_no_parts DROP CONSTRAINT check_a;
DROP TABLE parted_no_parts;
-- cannot drop inherited NOT NULL or check constraints from partition
ALTER TABLE list_parted2 ALTER b SET NOT NULL, ADD CONSTRAINT check_a2 CHECK (a > 0);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 453799abed4..cba15ebfec8 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2815,22 +2815,23 @@ ALTER TABLE part_2 DROP COLUMN b;
ALTER TABLE part_2 RENAME COLUMN b to c;
ALTER TABLE part_2 ALTER COLUMN b TYPE text;
--- cannot add/drop NOT NULL or check constraints to *only* the parent, when
+-- cannot add NOT NULL or check constraints to *only* the parent, when
-- partitions exist
ALTER TABLE ONLY list_parted2 ALTER b SET NOT NULL;
ALTER TABLE ONLY list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
+-- dropping them is ok though
ALTER TABLE list_parted2 ALTER b SET NOT NULL;
ALTER TABLE ONLY list_parted2 ALTER b DROP NOT NULL;
ALTER TABLE list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_b;
+-- ... and the partitions should still have both
+\d+ part_2
-- It's alright though, if no partitions are yet created
CREATE TABLE parted_no_parts (a int) PARTITION BY LIST (a);
ALTER TABLE ONLY parted_no_parts ALTER a SET NOT NULL;
ALTER TABLE ONLY parted_no_parts ADD CONSTRAINT check_a CHECK (a > 0);
-ALTER TABLE ONLY parted_no_parts ALTER a DROP NOT NULL;
-ALTER TABLE ONLY parted_no_parts DROP CONSTRAINT check_a;
DROP TABLE parted_no_parts;
-- cannot drop inherited NOT NULL or check constraints from partition