On 2025-Jun-27, Fujii Masao wrote: > On 2025/06/27 22:30, Álvaro Herrera wrote: > > On 2025-06-27, Fujii Masao wrote: > > > > > To make this distinction, I just started thinking it's better to raise > > > the error > > > in ATExecAlterConstraint() rather than in gram.y. I've attached a draft > > > patch that > > > follows this approach. > > > > Hmm I don't like this very much, it feels very kludgy. I think if we want > > to consider this in scope for pg18 I would much prefer to use the patch I > > mentioned near the beginning of the thread. > > Are you referring to v2-0001-trial.patch proposed at [1]?
Yeah. > This patch raises an error if ENFORCED, NOT ENFORCED, INHERIT, or NO INHERIT > is specified. But those options are currently accepted, so these checks seem > unnecessary for now. Sure, the patch was developed before those options were added, so I meant to reference the general approach rather than the specifics. > Also, the patch raises an error for NOT VALID after calling processCASbits(), > there's no need to call processCASbits() in the first place. It would be > cleaner to check for NOT VALID and raise an error before calling it. Yeah, I remember having this in mind when I read Amul's patch back in the day, too. > Jian He already proposed this approach in a patch at [2]. > > It looks like Jian's patch at [2] is an updated version of the one you > referred to, > so you may agree with that approach. Thought? Ah, okay, yes I like this approach better. > 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). -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Entristecido, Wutra (canción de Las Barreras) echa a Freyr a rodar y a nosotros al mar"
>From 10b9ac219ba4d01dcdf83c2fb9d7d6b344bd74f2 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 v4] 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 | 9 +++++++++ src/test/regress/expected/constraints.out | 5 +++++ src/test/regress/expected/foreign_key.out | 2 +- src/test/regress/expected/triggers.out | 9 +++++++++ src/test/regress/sql/constraints.sql | 3 +++ src/test/regress/sql/triggers.sql | 7 +++++++ 6 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 50f53159d58..8a1712a099d 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, @@ -6036,6 +6040,11 @@ CreateTrigStmt: { CreateTrigStmt *n = makeNode(CreateTrigStmt); + if (($11 & CAS_NOT_VALID) != 0) + ereport(ERROR, + errmsg("cannot create a NOT VALID constraint trigger"), + parser_errposition(@11)); + n->replace = $2; if (n->replace) /* not supported, see CreateTrigger */ ereport(ERROR, 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/expected/triggers.out b/src/test/regress/expected/triggers.out index 2bf0e77d61e..8f0c6cd1843 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -2280,6 +2280,15 @@ 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: cannot create a NOT VALID constraint trigger +LINE 2: after insert on foo not valid + ^ +-- -- 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/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; -- diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 9ffd318385f..c85d0da665c 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1576,6 +1576,13 @@ 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 (); + -- -- Constraint triggers and partitioned tables create table parted_constr_ancestor (a int, b text) -- 2.39.5
