On 2025-Jun-30, Álvaro Herrera wrote:
> > Just one note: Jian's patch doesn't handle the same issue for TRIGGER
> > case, so that part might still need to be addressed.
>
> Okay, here's my take on this, wherein I reworded the proposed error
> message. I also handled the NOT VALID case of a constraint trigger; maybe my
> patch is too focused on that specific bit and instead we should handle
> also NO INHERIT and NOT ENFORCED cases, not really sure (it's certainly
> not an important part of this patch).
For ease of review, here's the three patches. 0001 solves the main
problem with ALTER objtype ALTER CONSTRAINT NOT VALID.
I propose to put 0001 in 18 and 19, and leave 0002 and 0003 (as a single
commit) for 19 only, since it's been like that for ages and there have
been zero complaints before my own in the other thread. I put 0002 as a
separate one just for review, to show that these errors we throw are
nothing new: these commands would also fail if we don't patch this code,
they're just using bad grammar, which is then fixed by 0003.
I think I should list Amul as the fourth co-author of 0001. That would
make the longest list of coauthorship for a patch that small. Or I
could just say: "Author: Postgres Global Development Group".
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
>From 484b22d971be13d0c7f5a88d40aff85720a85203 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@kurilemu.de>
Date: Mon, 30 Jun 2025 17:07:04 +0200
Subject: [PATCH v5 1/3] Fix error message for ALTER CONSTRAINT ... NOT VALID
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Trying to alter a constraint so that it becomes NOT VALID results in an
error that assumes the constraint is a foreign key. This is potentially
wrong, so give a more generic error message.
While at it, give CREATE CONSTRAINT TRIGGER a better error message as
well.
Co-authored-by: jian he <jian.universal...@gmail.com>
Co-authored-by: Fujii Masao <masao.fu...@oss.nttdata.com>
Co-authored-by: Álvaro Herrera <alvhe...@kurilemu.de>
Discussion: https://postgr.es/m/CACJufxHSp2puxP=q8ztugl1f+heapnzqfbzy5zngujugwjb...@mail.gmail.com
---
src/backend/parser/gram.y | 4 ++++
src/test/regress/expected/constraints.out | 5 +++++
src/test/regress/expected/foreign_key.out | 2 +-
src/test/regress/sql/constraints.sql | 3 +++
4 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 50f53159d58..527c7ea75e3 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2668,6 +2668,10 @@ alter_table_cmd:
c->alterDeferrability = true;
if ($4 & CAS_NO_INHERIT)
c->alterInheritability = true;
+ if ($4 & CAS_NOT_VALID)
+ ereport(ERROR,
+ errmsg("constraints cannot be altered to be NOT VALID"),
+ parser_errposition(@4));
processCASbits($4, @4, "FOREIGN KEY",
&c->deferrable,
&c->initdeferred,
diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out
index b5592617d97..ccea883cffd 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -748,6 +748,11 @@ ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key ENFORCED;
ERROR: cannot alter enforceability of constraint "unique_tbl_i_key" of relation "unique_tbl"
ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key NOT ENFORCED;
ERROR: cannot alter enforceability of constraint "unique_tbl_i_key" of relation "unique_tbl"
+-- can't make an existing constraint NOT VALID
+ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key NOT VALID;
+ERROR: constraints cannot be altered to be NOT VALID
+LINE 1: ...ABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key NOT VALID;
+ ^
DROP TABLE unique_tbl;
--
-- EXCLUDE constraints
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 6a8f3959345..f9bd252444f 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1359,7 +1359,7 @@ LINE 1: ...e ALTER CONSTRAINT fktable_fk_fkey NOT DEFERRABLE INITIALLY ...
ALTER 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
+ERROR: constraints cannot be altered to be NOT VALID
LINE 1: ...ER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey NOT VALID;
^
ALTER TABLE fktable ALTER CONSTRAINT fktable_fk_fkey ENFORCED NOT ENFORCED;
diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql
index 12668f0e0ce..7487723ab84 100644
--- a/src/test/regress/sql/constraints.sql
+++ b/src/test/regress/sql/constraints.sql
@@ -537,6 +537,9 @@ CREATE TABLE UNIQUE_NOTEN_TBL(i int UNIQUE NOT ENFORCED);
ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key ENFORCED;
ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key NOT ENFORCED;
+-- can't make an existing constraint NOT VALID
+ALTER TABLE unique_tbl ALTER CONSTRAINT unique_tbl_i_key NOT VALID;
+
DROP TABLE unique_tbl;
--
--
2.39.5
>From 1a00291b39e915263e38d9841683d11a050970b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@kurilemu.de>
Date: Mon, 30 Jun 2025 20:15:08 +0200
Subject: [PATCH v5 2/3] Tests to illustrate bogus messages in CREATE
CONSTRAINT TRIGGER
---
src/test/regress/expected/triggers.out | 21 +++++++++++++++++++++
src/test/regress/sql/triggers.sql | 13 +++++++++++++
2 files changed, 34 insertions(+)
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 2bf0e77d61e..626fabf41be 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2280,6 +2280,27 @@ select * from parted;
drop table parted;
drop function parted_trigfunc();
--
+-- Constraint triggers
+--
+create constraint trigger crtr
+ after insert on foo not valid
+ for each row execute procedure foo ();
+ERROR: TRIGGER constraints cannot be marked NOT VALID
+LINE 2: after insert on foo not valid
+ ^
+create constraint trigger crtr
+ after insert on foo no inherit
+ for each row execute procedure foo ();
+ERROR: TRIGGER constraints cannot be marked NO INHERIT
+LINE 2: after insert on foo no inherit
+ ^
+create constraint trigger crtr
+ after insert on foo not enforced
+ for each row execute procedure foo ();
+ERROR: TRIGGER constraints cannot be marked NOT ENFORCED
+LINE 2: after insert on foo not enforced
+ ^
+--
-- Constraint triggers and partitioned tables
create table parted_constr_ancestor (a int, b text)
partition by range (b);
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 9ffd318385f..75a334ebad5 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1576,6 +1576,19 @@ select * from parted;
drop table parted;
drop function parted_trigfunc();
+--
+-- Constraint triggers
+--
+create constraint trigger crtr
+ after insert on foo not valid
+ for each row execute procedure foo ();
+create constraint trigger crtr
+ after insert on foo no inherit
+ for each row execute procedure foo ();
+create constraint trigger crtr
+ after insert on foo not enforced
+ for each row execute procedure foo ();
+
--
-- Constraint triggers and partitioned tables
create table parted_constr_ancestor (a int, b text)
--
2.39.5
>From 072fcc83a92a16982fb93957dbd6ba807f1de5e6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@kurilemu.de>
Date: Mon, 30 Jun 2025 20:15:17 +0200
Subject: [PATCH v5 3/3] fix grammar
---
src/backend/parser/gram.y | 16 ++++++++++++++++
src/test/regress/expected/triggers.out | 6 +++---
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 527c7ea75e3..f66fbd20a4d 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -6040,6 +6040,22 @@ CreateTrigStmt:
{
CreateTrigStmt *n = makeNode(CreateTrigStmt);
+ if (($11 & CAS_NOT_VALID) != 0)
+ ereport(ERROR,
+ errmsg("constraint triggers cannot be marked %s",
+ "NOT VALID"),
+ parser_errposition(@11));
+ if (($11 & CAS_NO_INHERIT) != 0)
+ ereport(ERROR,
+ errmsg("constraint triggers cannot be marked %s",
+ "NO INHERIT"),
+ parser_errposition(@11));
+ if (($11 & CAS_NOT_ENFORCED) != 0)
+ ereport(ERROR,
+ errmsg("constraint triggers cannot be marked %s",
+ "NOT ENFORCED"),
+ parser_errposition(@11));
+
n->replace = $2;
if (n->replace) /* not supported, see CreateTrigger */
ereport(ERROR,
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 626fabf41be..69c8dbf0084 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2285,19 +2285,19 @@ drop function parted_trigfunc();
create constraint trigger crtr
after insert on foo not valid
for each row execute procedure foo ();
-ERROR: TRIGGER constraints cannot be marked NOT VALID
+ERROR: constraint triggers cannot be marked NOT VALID
LINE 2: after insert on foo not valid
^
create constraint trigger crtr
after insert on foo no inherit
for each row execute procedure foo ();
-ERROR: TRIGGER constraints cannot be marked NO INHERIT
+ERROR: constraint triggers cannot be marked NO INHERIT
LINE 2: after insert on foo no inherit
^
create constraint trigger crtr
after insert on foo not enforced
for each row execute procedure foo ();
-ERROR: TRIGGER constraints cannot be marked NOT ENFORCED
+ERROR: constraint triggers cannot be marked NOT ENFORCED
LINE 2: after insert on foo not enforced
^
--
--
2.39.5
