Skip to content

Commit 4dea33c

Browse files
alvherrealvherre
committed
Don't disallow DROP of constraints ONLY on partitioned tables
This restriction seems to have come about due to some fuzzy thinking: in commit 9139aa1 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 9139aa1, previously not registered)
1 parent 4c7cd07 commit 4dea33c

File tree

4 files changed

+22
-48
lines changed

4 files changed

+22
-48
lines changed

‎doc/src/sgml/ref/alter_table.sgml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,9 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
251251
table. Even if there is no <literal>NOT NULL</literal> constraint on the
252252
parent, such a constraint can still be added to individual partitions,
253253
if desired; that is, the children can disallow nulls even if the parent
254-
allows them, but not the other way around.
254+
allows them, but not the other way around. It is also possible to drop
255+
the <literal>NOT NULL</literal> constraint from <literal>ONLY</literal>
256+
the parent table, which does not remove it from the children.
255257
</para>
256258
</listitem>
257259
</varlistentry>

‎src/backend/commands/tablecmds.c

Lines changed: 1 addition & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,6 @@ static bool check_for_column_name_collision(Relation rel, const char *colname,
447447
bool if_not_exists);
448448
static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid typid);
449449
static void add_column_collation_dependency(Oid relid, int32 attnum, Oid collid);
450-
static void ATPrepDropNotNull(Relation rel, bool recurse, bool recursing);
451450
static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName, LOCKMODE lockmode);
452451
static void ATPrepSetNotNull(List **wqueue, Relation rel,
453452
AlterTableCmd *cmd, bool recurse, bool recursing,
@@ -4858,7 +4857,6 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
48584857
case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */
48594858
ATSimplePermissions(cmd->subtype, rel,
48604859
ATT_TABLE | ATT_PARTITIONED_TABLE | ATT_FOREIGN_TABLE);
4861-
ATPrepDropNotNull(rel, recurse, recursing);
48624860
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode, context);
48634861
pass = AT_PASS_DROP;
48644862
break;
@@ -7498,29 +7496,7 @@ add_column_collation_dependency(Oid relid, int32 attnum, Oid collid)
74987496

74997497
/*
75007498
* ALTER TABLE ALTER COLUMN DROP NOT NULL
7501-
*/
7502-
7503-
static void
7504-
ATPrepDropNotNull(Relation rel, bool recurse, bool recursing)
7505-
{
7506-
/*
7507-
* If the parent is a partitioned table, like check constraints, we do not
7508-
* support removing the NOT NULL while partitions exist.
7509-
*/
7510-
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
7511-
{
7512-
PartitionDesc partdesc = RelationGetPartitionDesc(rel, true);
7513-
7514-
Assert(partdesc != NULL);
7515-
if (partdesc->nparts > 0 && !recurse && !recursing)
7516-
ereport(ERROR,
7517-
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
7518-
errmsg("cannot remove constraint from only the partitioned table when partitions exist"),
7519-
errhint("Do not specify the ONLY keyword.")));
7520-
}
7521-
}
7522-
7523-
/*
7499+
*
75247500
* Return the address of the modified column. If the column was already
75257501
* nullable, InvalidObjectAddress is returned.
75267502
*/
@@ -12713,18 +12689,6 @@ ATExecDropConstraint(Relation rel, const char *constrName,
1271312689
else
1271412690
children = NIL;
1271512691

12716-
/*
12717-
* For a partitioned table, if partitions exist and we are told not to
12718-
* recurse, it's a user error. It doesn't make sense to have a constraint
12719-
* be defined only on the parent, especially if it's a partitioned table.
12720-
*/
12721-
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
12722-
children != NIL && !recurse)
12723-
ereport(ERROR,
12724-
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
12725-
errmsg("cannot remove constraint from only the partitioned table when partitions exist"),
12726-
errhint("Do not specify the ONLY keyword.")));
12727-
1272812692
foreach_oid(childrelid, children)
1272912693
{
1273012694
Relation childrel;

‎src/test/regress/expected/alter_table.out

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4401,28 +4401,35 @@ ALTER TABLE part_2 RENAME COLUMN b to c;
44014401
ERROR: cannot rename inherited column "b"
44024402
ALTER TABLE part_2 ALTER COLUMN b TYPE text;
44034403
ERROR: cannot alter inherited column "b"
4404-
-- cannot add/drop NOT NULL or check constraints to *only* the parent, when
4404+
-- cannot add NOT NULL or check constraints to *only* the parent, when
44054405
-- partitions exist
44064406
ALTER TABLE ONLY list_parted2 ALTER b SET NOT NULL;
44074407
ERROR: constraint must be added to child tables too
44084408
DETAIL: Column "b" of relation "part_2" is not already NOT NULL.
44094409
HINT: Do not specify the ONLY keyword.
44104410
ALTER TABLE ONLY list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
44114411
ERROR: constraint must be added to child tables too
4412+
-- dropping them is ok though
44124413
ALTER TABLE list_parted2 ALTER b SET NOT NULL;
44134414
ALTER TABLE ONLY list_parted2 ALTER b DROP NOT NULL;
4414-
ERROR: cannot remove constraint from only the partitioned table when partitions exist
4415-
HINT: Do not specify the ONLY keyword.
44164415
ALTER TABLE list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
44174416
ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_b;
4418-
ERROR: cannot remove constraint from only the partitioned table when partitions exist
4419-
HINT: Do not specify the ONLY keyword.
4417+
-- ... and the partitions should still have both
4418+
\d+ part_2
4419+
Table "public.part_2"
4420+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
4421+
--------+--------------+-----------+----------+---------+----------+--------------+-------------
4422+
a | integer | | | | plain | |
4423+
b | character(1) | | not null | | extended | |
4424+
Partition of: list_parted2 FOR VALUES IN (2)
4425+
Partition constraint: ((a IS NOT NULL) AND (a = 2))
4426+
Check constraints:
4427+
"check_b" CHECK (b <> 'zz'::bpchar)
4428+
44204429
-- It's alright though, if no partitions are yet created
44214430
CREATE TABLE parted_no_parts (a int) PARTITION BY LIST (a);
44224431
ALTER TABLE ONLY parted_no_parts ALTER a SET NOT NULL;
44234432
ALTER TABLE ONLY parted_no_parts ADD CONSTRAINT check_a CHECK (a > 0);
4424-
ALTER TABLE ONLY parted_no_parts ALTER a DROP NOT NULL;
4425-
ALTER TABLE ONLY parted_no_parts DROP CONSTRAINT check_a;
44264433
DROP TABLE parted_no_parts;
44274434
-- cannot drop inherited NOT NULL or check constraints from partition
44284435
ALTER TABLE list_parted2 ALTER b SET NOT NULL, ADD CONSTRAINT check_a2 CHECK (a > 0);

‎src/test/regress/sql/alter_table.sql

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2815,22 +2815,23 @@ ALTER TABLE part_2 DROP COLUMN b;
28152815
ALTER TABLE part_2 RENAME COLUMN b to c;
28162816
ALTER TABLE part_2 ALTER COLUMN b TYPE text;
28172817

2818-
-- cannot add/drop NOT NULL or check constraints to *only* the parent, when
2818+
-- cannot add NOT NULL or check constraints to *only* the parent, when
28192819
-- partitions exist
28202820
ALTER TABLE ONLY list_parted2 ALTER b SET NOT NULL;
28212821
ALTER TABLE ONLY list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
28222822

2823+
-- dropping them is ok though
28232824
ALTER TABLE list_parted2 ALTER b SET NOT NULL;
28242825
ALTER TABLE ONLY list_parted2 ALTER b DROP NOT NULL;
28252826
ALTER TABLE list_parted2 ADD CONSTRAINT check_b CHECK (b <> 'zz');
28262827
ALTER TABLE ONLY list_parted2 DROP CONSTRAINT check_b;
2828+
-- ... and the partitions should still have both
2829+
\d+ part_2
28272830

28282831
-- It's alright though, if no partitions are yet created
28292832
CREATE TABLE parted_no_parts (a int) PARTITION BY LIST (a);
28302833
ALTER TABLE ONLY parted_no_parts ALTER a SET NOT NULL;
28312834
ALTER TABLE ONLY parted_no_parts ADD CONSTRAINT check_a CHECK (a > 0);
2832-
ALTER TABLE ONLY parted_no_parts ALTER a DROP NOT NULL;
2833-
ALTER TABLE ONLY parted_no_parts DROP CONSTRAINT check_a;
28342835
DROP TABLE parted_no_parts;
28352836

28362837
-- cannot drop inherited NOT NULL or check constraints from partition

0 commit comments

Comments
 (0)