simplifying grammar for ALTER CONSTRAINT .. SET [NO] INHERIT
With commit f4e53e10b6ce we introduced a way to flip the NO INHERIT bit
on not-null constraints. However, because of the way the grammar
dealt with ALTER CONSTRAINT, we were too blind to see a way to implement
it using the existing production. It turns out that we can remove it,
so the commands would be
ALTER TABLE tab ALTER CONSTRAINT constr INHERIT
ALTER TABLE tab ALTER CONSTRAINT constr NO INHERIT
i.e. the word SET is no longer needed.
Do people find this better?
A proposed patch is attached. One thing not fully clear to me is that
now the entry for ALTER CONSTRAINT in the sgml docs is awkward, because
it describes the INHERIT flag separate from the deferrability flags.
After this patch, it's a bit more obvious that the error messages we're
throwing now aren't ideal:
55432 18devel 2953943=# create table foo (a int not null);
CREATE TABLE
55432 18devel 2953943=# alter table foo alter constraint foo_a_not_null no inherit deferrable;
ERROR: constraint "foo_a_not_null" of relation "foo" is not a foreign key constraint
55432 18devel 2953943=# create table bar (a int primary key references bar);
CREATE TABLE
55432 18devel 2953943=# alter table bar alter constraint bar_a_fkey deferrable no inherit;
ERROR: constraint "bar_a_fkey" of relation "bar" is not a not-null constraint
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Saca el libro que tu religión considere como el indicado para encontrar la
oración que traiga paz a tu alma. Luego rebootea el computador
y ve si funciona" (Carlos Duclós)
On 2025-Mar-25, Álvaro Herrera wrote:
With commit f4e53e10b6ce we introduced a way to flip the NO INHERIT bit
on not-null constraints. However, because of the way the grammar
dealt with ALTER CONSTRAINT, we were too blind to see a way to implement
it using the existing production.
Patch attached.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Someone said that it is at least an order of magnitude more work to do
production software than a prototype. I think he is wrong by at least
an order of magnitude." (Brian Kernighan)
Attachments:
0001-change-syntax-from-SET-NO-INHERIT-to-just-NO-INHERIT.patchtext/x-diff; charset=utf-8Download
From ad94ec630c4d4daa3ca2e2983ca2087e089ae120 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@alvh.no-ip.org>
Date: Tue, 25 Mar 2025 17:03:32 +0100
Subject: [PATCH] change syntax from SET NO INHERIT to just NO INHERIT
---
doc/src/sgml/ref/alter_table.sgml | 6 ++---
src/backend/parser/gram.y | 26 ++++++--------------
src/test/regress/expected/foreign_key.out | 4 +--
src/test/regress/expected/inherit.out | 30 +++++++++++------------
src/test/regress/sql/inherit.sql | 30 +++++++++++------------
5 files changed, 42 insertions(+), 54 deletions(-)
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 4f15b89a98f..11d1bc7dbe1 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -59,7 +59,7 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="parameter">name</replaceable>
ADD <replaceable class="parameter">table_constraint</replaceable> [ NOT VALID ]
ADD <replaceable class="parameter">table_constraint_using_index</replaceable>
ALTER CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]
- ALTER CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> SET [ INHERIT | NO INHERIT ]
+ ALTER CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> [ INHERIT | NO INHERIT ]
VALIDATE CONSTRAINT <replaceable class="parameter">constraint_name</replaceable>
DROP CONSTRAINT [ IF EXISTS ] <replaceable class="parameter">constraint_name</replaceable> [ RESTRICT | CASCADE ]
DISABLE TRIGGER [ <replaceable class="parameter">trigger_name</replaceable> | ALL | USER ]
@@ -564,8 +564,8 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
</varlistentry>
<varlistentry id="sql-altertable-desc-alter-constraint-inherit">
- <term><literal>ALTER CONSTRAINT ... SET INHERIT</literal></term>
- <term><literal>ALTER CONSTRAINT ... SET NO INHERIT</literal></term>
+ <term><literal>ALTER CONSTRAINT ... INHERIT</literal></term>
+ <term><literal>ALTER CONSTRAINT ... NO INHERIT</literal></term>
<listitem>
<para>
These forms modify a inheritable constraint so that it becomes not
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 271ae26cbaf..0fc502a3a40 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2662,15 +2662,19 @@ alter_table_cmd:
n->subtype = AT_AlterConstraint;
n->def = (Node *) c;
c->conname = $3;
- c->alterDeferrability = true;
+ if ($4 & (CAS_DEFERRABLE | CAS_NOT_DEFERRABLE |
+ CAS_INITIALLY_DEFERRED | CAS_INITIALLY_IMMEDIATE))
+ c->alterDeferrability = true;
+ if ($4 & CAS_NO_INHERIT)
+ c->alterInheritability = true;
processCASbits($4, @4, "FOREIGN KEY",
&c->deferrable,
&c->initdeferred,
- NULL, NULL, NULL, yyscanner);
+ NULL, NULL, &c->noinherit, yyscanner);
$$ = (Node *) n;
}
- /* ALTER TABLE <name> ALTER CONSTRAINT SET INHERIT */
- | ALTER CONSTRAINT name SET INHERIT
+ /* ALTER TABLE <name> ALTER CONSTRAINT INHERIT */
+ | ALTER CONSTRAINT name INHERIT
{
AlterTableCmd *n = makeNode(AlterTableCmd);
ATAlterConstraint *c = makeNode(ATAlterConstraint);
@@ -2681,20 +2685,6 @@ alter_table_cmd:
c->alterInheritability = true;
c->noinherit = false;
- $$ = (Node *) n;
- }
- /* ALTER TABLE <name> ALTER CONSTRAINT SET NO INHERIT */
- | ALTER CONSTRAINT name SET NO INHERIT
- {
- AlterTableCmd *n = makeNode(AlterTableCmd);
- ATAlterConstraint *c = makeNode(ATAlterConstraint);
-
- n->subtype = AT_AlterConstraint;
- n->def = (Node *) c;
- c->conname = $3;
- c->alterInheritability = true;
- c->noinherit = true;
-
$$ = (Node *) n;
}
/* ALTER TABLE <name> VALIDATE CONSTRAINT ... */
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 6a3374d5152..7f678349a8e 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1284,9 +1284,7 @@ ERROR: constraint declared INITIALLY DEFERRED must be DEFERRABLE
LINE 1: ...e ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY ...
^
ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NO INHERIT;
-ERROR: FOREIGN KEY constraints cannot be marked NO INHERIT
-LINE 1: ...ER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NO INHERIT...
- ^
+ERROR: constraint "fktable_fk_fkey" of relation "fktable" is not a not-null constraint
ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT VALID;
ERROR: FOREIGN KEY constraints cannot be marked NOT VALID
LINE 1: ...ER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT VALID;
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index e671975a281..58461e7f53f 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -2754,7 +2754,7 @@ alter table inh_nn2 inherit inh_nn1;
create table inh_nn3 (f4 float) inherits (inh_nn2);
create table inh_nn4 (f5 int, f4 float, f2 text, f3 int, f1 int);
alter table inh_nn4 inherit inh_nn2, inherit inh_nn1, inherit inh_nn3;
-alter table inh_nn1 alter constraint inh_nn1_f1_not_null set inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_not_null inherit;
select conrelid::regclass, conname, conkey, coninhcount, conislocal, connoinherit
from pg_constraint where contype = 'n' and
conrelid::regclass::text in ('inh_nn1', 'inh_nn2', 'inh_nn3', 'inh_nn4')
@@ -2767,8 +2767,8 @@ select conrelid::regclass, conname, conkey, coninhcount, conislocal, connoinheri
inh_nn4 | inh_nn1_f1_not_null | {5} | 3 | f | f
(4 rows)
--- ALTER CONSTRAINT SET NO INHERIT should work on top-level constraints
-alter table inh_nn1 alter constraint inh_nn1_f1_not_null set no inherit;
+-- ALTER CONSTRAINT NO INHERIT should work on top-level constraints
+alter table inh_nn1 alter constraint inh_nn1_f1_not_null no inherit;
select conrelid::regclass, conname, conkey, coninhcount, conislocal, connoinherit
from pg_constraint where contype = 'n' and
conrelid::regclass::text in ('inh_nn1', 'inh_nn2', 'inh_nn3', 'inh_nn4')
@@ -2799,15 +2799,15 @@ drop table inh_nn1, inh_nn2, inh_nn3, inh_nn4;
create table inh_nn1 (f1 int not null no inherit);
create table inh_nn2 (f2 text, f3 int) inherits (inh_nn1);
insert into inh_nn2 values(NULL, 'sample', 1);
-alter table inh_nn1 alter constraint inh_nn1_f1_not_null set inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_not_null inherit;
ERROR: column "f1" of relation "inh_nn2" contains null values
delete from inh_nn2;
create table inh_nn3 () inherits (inh_nn2);
create table inh_nn4 () inherits (inh_nn1, inh_nn2);
NOTICE: merging multiple inherited definitions of column "f1"
alter table inh_nn1 -- test multicommand alter table while at it
- alter constraint inh_nn1_f1_not_null set inherit,
- alter constraint inh_nn1_f1_not_null set no inherit;
+ alter constraint inh_nn1_f1_not_null inherit,
+ alter constraint inh_nn1_f1_not_null no inherit;
select conrelid::regclass, conname, coninhcount, conislocal, connoinherit
from pg_constraint where contype = 'n' and
conrelid::regclass::text in ('inh_nn1', 'inh_nn2', 'inh_nn3', 'inh_nn4')
@@ -2837,10 +2837,10 @@ select conrelid::regclass, conname, conkey, coninhcount, conislocal, connoinheri
(2 rows)
-- error: inh_nn3 has an incompatible NO INHERIT constraint
-alter table inh_nn1 alter constraint inh_nn1_f1_not_null set inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_not_null inherit;
ERROR: cannot change NO INHERIT status of NOT NULL constraint "nn3_f1" on relation "inh_nn3"
-alter table inh_nn3 alter constraint nn3_f1 set inherit;
-alter table inh_nn1 alter constraint inh_nn1_f1_not_null set inherit; -- now it works
+alter table inh_nn3 alter constraint nn3_f1 inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_not_null inherit; -- now it works
select conrelid::regclass, conname, conkey, coninhcount, conislocal, connoinherit
from pg_constraint where contype = 'n' and
conrelid::regclass::text in ('inh_nn1', 'inh_nn2', 'inh_nn3')
@@ -2853,21 +2853,21 @@ select conrelid::regclass, conname, conkey, coninhcount, conislocal, connoinheri
(3 rows)
drop table inh_nn1, inh_nn2, inh_nn3;
--- Negative scenarios for alter constraint .. set inherit.
+-- Negative scenarios for alter constraint .. inherit.
create table inh_nn1 (f1 int check(f1 > 5) primary key references inh_nn1, f2 int not null);
-- constraints other than not-null are not supported
-alter table inh_nn1 alter constraint inh_nn1_f1_check set inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_check inherit;
ERROR: constraint "inh_nn1_f1_check" of relation "inh_nn1" is not a not-null constraint
-alter table inh_nn1 alter constraint inh_nn1_pkey set inherit;
+alter table inh_nn1 alter constraint inh_nn1_pkey inherit;
ERROR: constraint "inh_nn1_pkey" of relation "inh_nn1" is not a not-null constraint
-alter table inh_nn1 alter constraint inh_nn1_f1_fkey set inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_fkey inherit;
ERROR: constraint "inh_nn1_f1_fkey" of relation "inh_nn1" is not a not-null constraint
-- try to drop a nonexistant constraint
-alter table inh_nn1 alter constraint foo set inherit;
+alter table inh_nn1 alter constraint foo inherit;
ERROR: constraint "foo" of relation "inh_nn1" does not exist
-- Can't modify inheritability of inherited constraints
create table inh_nn2 () inherits (inh_nn1);
-alter table inh_nn2 alter constraint inh_nn1_f2_not_null set no inherit;
+alter table inh_nn2 alter constraint inh_nn1_f2_not_null no inherit;
ERROR: cannot alter inherited constraint "inh_nn1_f2_not_null" on relation "inh_nn2"
drop table inh_nn1, inh_nn2;
--
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 4e73c70495c..8df0f8af08a 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -1099,13 +1099,13 @@ alter table inh_nn2 inherit inh_nn1;
create table inh_nn3 (f4 float) inherits (inh_nn2);
create table inh_nn4 (f5 int, f4 float, f2 text, f3 int, f1 int);
alter table inh_nn4 inherit inh_nn2, inherit inh_nn1, inherit inh_nn3;
-alter table inh_nn1 alter constraint inh_nn1_f1_not_null set inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_not_null inherit;
select conrelid::regclass, conname, conkey, coninhcount, conislocal, connoinherit
from pg_constraint where contype = 'n' and
conrelid::regclass::text in ('inh_nn1', 'inh_nn2', 'inh_nn3', 'inh_nn4')
order by 2, 1;
--- ALTER CONSTRAINT SET NO INHERIT should work on top-level constraints
-alter table inh_nn1 alter constraint inh_nn1_f1_not_null set no inherit;
+-- ALTER CONSTRAINT NO INHERIT should work on top-level constraints
+alter table inh_nn1 alter constraint inh_nn1_f1_not_null no inherit;
select conrelid::regclass, conname, conkey, coninhcount, conislocal, connoinherit
from pg_constraint where contype = 'n' and
conrelid::regclass::text in ('inh_nn1', 'inh_nn2', 'inh_nn3', 'inh_nn4')
@@ -1122,13 +1122,13 @@ drop table inh_nn1, inh_nn2, inh_nn3, inh_nn4;
create table inh_nn1 (f1 int not null no inherit);
create table inh_nn2 (f2 text, f3 int) inherits (inh_nn1);
insert into inh_nn2 values(NULL, 'sample', 1);
-alter table inh_nn1 alter constraint inh_nn1_f1_not_null set inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_not_null inherit;
delete from inh_nn2;
create table inh_nn3 () inherits (inh_nn2);
create table inh_nn4 () inherits (inh_nn1, inh_nn2);
alter table inh_nn1 -- test multicommand alter table while at it
- alter constraint inh_nn1_f1_not_null set inherit,
- alter constraint inh_nn1_f1_not_null set no inherit;
+ alter constraint inh_nn1_f1_not_null inherit,
+ alter constraint inh_nn1_f1_not_null no inherit;
select conrelid::regclass, conname, coninhcount, conislocal, connoinherit
from pg_constraint where contype = 'n' and
conrelid::regclass::text in ('inh_nn1', 'inh_nn2', 'inh_nn3', 'inh_nn4')
@@ -1144,26 +1144,26 @@ select conrelid::regclass, conname, conkey, coninhcount, conislocal, connoinheri
conrelid::regclass::text in ('inh_nn1', 'inh_nn2', 'inh_nn3')
order by 2, 1;
-- error: inh_nn3 has an incompatible NO INHERIT constraint
-alter table inh_nn1 alter constraint inh_nn1_f1_not_null set inherit;
-alter table inh_nn3 alter constraint nn3_f1 set inherit;
-alter table inh_nn1 alter constraint inh_nn1_f1_not_null set inherit; -- now it works
+alter table inh_nn1 alter constraint inh_nn1_f1_not_null inherit;
+alter table inh_nn3 alter constraint nn3_f1 inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_not_null inherit; -- now it works
select conrelid::regclass, conname, conkey, coninhcount, conislocal, connoinherit
from pg_constraint where contype = 'n' and
conrelid::regclass::text in ('inh_nn1', 'inh_nn2', 'inh_nn3')
order by 2, 1;
drop table inh_nn1, inh_nn2, inh_nn3;
--- Negative scenarios for alter constraint .. set inherit.
+-- Negative scenarios for alter constraint .. inherit.
create table inh_nn1 (f1 int check(f1 > 5) primary key references inh_nn1, f2 int not null);
-- constraints other than not-null are not supported
-alter table inh_nn1 alter constraint inh_nn1_f1_check set inherit;
-alter table inh_nn1 alter constraint inh_nn1_pkey set inherit;
-alter table inh_nn1 alter constraint inh_nn1_f1_fkey set inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_check inherit;
+alter table inh_nn1 alter constraint inh_nn1_pkey inherit;
+alter table inh_nn1 alter constraint inh_nn1_f1_fkey inherit;
-- try to drop a nonexistant constraint
-alter table inh_nn1 alter constraint foo set inherit;
+alter table inh_nn1 alter constraint foo inherit;
-- Can't modify inheritability of inherited constraints
create table inh_nn2 () inherits (inh_nn1);
-alter table inh_nn2 alter constraint inh_nn1_f2_not_null set no inherit;
+alter table inh_nn2 alter constraint inh_nn1_f2_not_null no inherit;
drop table inh_nn1, inh_nn2;
--
2.39.5
On 25.03.25 17:02, Álvaro Herrera wrote:
With commit f4e53e10b6ce we introduced a way to flip the NO INHERIT bit
on not-null constraints. However, because of the way the grammar
dealt with ALTER CONSTRAINT, we were too blind to see a way to implement
it using the existing production. It turns out that we can remove it,
so the commands would beALTER TABLE tab ALTER CONSTRAINT constr INHERIT
ALTER TABLE tab ALTER CONSTRAINT constr NO INHERITi.e. the word SET is no longer needed.
Do people find this better?
This seems better, considering that the SQL-standard syntax for ENFORCED is:
ALTER TABLE tab ALTER CONSTRAINT constr ENFORCED
ALTER TABLE tab ALTER CONSTRAINT constr NOT ENFORCED
also without "SET".
On Tue, Mar 25, 2025 at 9:32 PM Álvaro Herrera <alvherre@alvh.no-ip.org>
wrote:
With commit f4e53e10b6ce we introduced a way to flip the NO INHERIT bit
on not-null constraints. However, because of the way the grammar
dealt with ALTER CONSTRAINT, we were too blind to see a way to implement
it using the existing production. It turns out that we can remove it,
so the commands would beALTER TABLE tab ALTER CONSTRAINT constr INHERIT
ALTER TABLE tab ALTER CONSTRAINT constr NO INHERITi.e. the word SET is no longer needed.
Do people find this better?
Yes, I agree. As Peter said, it is now inline with other commands.
I have reviewed the patch and it looks good to me.
Since we are removing the SET keyword, how about removing that from the
below comment as well.
/*
* Propagate the change to children. For SET NO INHERIT, we don't
* recursively affect children, just the immediate level.
*/
This is the comment from ATExecAlterConstrInheritability().
On 2025-Mar-26, Suraj Kharage wrote:
Yes, I agree. As Peter said, it is now inline with other commands.
I have reviewed the patch and it looks good to me.
Thanks for reviewing!
Since we are removing the SET keyword, how about removing that from the
below comment as well./*
* Propagate the change to children. For SET NO INHERIT, we don't
* recursively affect children, just the immediate level.
*/This is the comment from ATExecAlterConstrInheritability().
Ah right. Fixed that and pushed.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/