mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Don't disallow DROP of constraints ONLY on partitioned tables

Browse Source 
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)
This commit is contained in:
Alvaro Herrera 2024-09-30 11:58:13 +02:00
parent 4c7cd07aa6
commit 4dea33ce76
No known key found for this signature in database
GPG Key ID: 1C20ACB9D5C564AE
4 changed files with 22 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/src/sgml/ref/alter_table.sgml
View File

@ -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>

38
src/backend/commands/tablecmds.c
View File

@ -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;

21
src/test/regress/expected/alter_table.out
View File

@ -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);

7
src/test/regress/sql/alter_table.sql
View File

@ -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