On 2025-Mar-17, jian he wrote: > hi. > I played around with it. > > current syntax, we don't need to deal with column constraint grammar. > like the following can fail directly: > create table t0(a int constraint nn not null a not valid); > we only support table constraint cases like: > alter table lp add constraint nc1 not null a not valid; > > since CREATE TABLE with invalid constraint does not make sense, so > we can just issue a warning. like: > create table t0(a int, constraint nn not null a not valid); > WARNING: CREATE TABLE NOT NULL NOT VALID CONSTRAINT WILL SET TO VALID > the wording needs to change...
Yeah, we discussed this elsewhere. I have an alpha-quality patch for that, but I wasn't too sure about it ... [1] https://postgr.es/m/cacjufxeqchnhn6m18jy1mqcgqq9gn9ofmeop47sdfde5b8w...@mail.gmail.com -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
>From 4eb5f71595753d035d5267aca93c8b2cca93b4a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvhe...@alvh.no-ip.org> Date: Tue, 14 Jan 2025 18:43:47 +0100 Subject: [PATCH] Throw warning if NOT VALID is given during CREATE TABLE Discussion: https://postgr.es/m/cacjufxeqchnhn6m18jy1mqcgqq9gn9ofmeop47sdfde5b8w...@mail.gmail.com --- src/backend/catalog/heap.c | 8 ++++++++ src/backend/commands/tablecmds.c | 7 +++++++ src/backend/parser/gram.y | 2 ++ src/include/nodes/parsenodes.h | 1 + src/test/regress/expected/alter_table.out | 1 + src/test/regress/expected/foreign_key.out | 3 ++- src/test/regress/sql/foreign_key.sql | 2 +- 7 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index bd3554c0bfd..0eb19f3c5cf 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -2572,6 +2572,12 @@ AddRelationNewConstraints(Relation rel, checknames = lappend(checknames, ccname); } + /* XXX improve error report */ + if (cdef->was_not_valid && cdef->initially_valid && cdef->is_enforced) + ereport(WARNING, + errcode(ERRCODE_WARNING), + errmsg("NOT VALID flag for constraint \"%s\" ignored", ccname)); + /* * OK, store it. */ @@ -3039,6 +3045,8 @@ AddRelationNotNullConstraints(Relation rel, List *constraints, nnnames); nnnames = lappend(nnnames, conname); + /* XXX if NOT VALID is given during CREATE TABLE, throw warning */ + StoreRelNotNull(rel, conname, attnum, true, true, inhcount, constr->is_no_inherit); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 129c97fdf28..b59f25b79eb 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10546,6 +10546,13 @@ addFkConstraint(addFkConstraintSides fkside, connoinherit = rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE; } + /* XXX improve error report */ + if (fkconstraint->was_not_valid && fkconstraint->initially_valid) + ereport(WARNING, + errcode(ERRCODE_WARNING), + errmsg("NOT VALID flag for constraint \"%s\" ignored", + fkconstraint->conname)); + /* * Record the FK constraint in pg_constraint. */ diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 271ae26cbaf..29b12a2ff46 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -4213,6 +4213,7 @@ ConstraintElem: NULL, NULL, &n->is_enforced, &n->skip_validation, &n->is_no_inherit, yyscanner); n->initially_valid = !n->skip_validation; + n->was_not_valid = n->skip_validation; $$ = (Node *) n; } | NOT NULL_P ColId ConstraintAttributeSpec @@ -4347,6 +4348,7 @@ ConstraintElem: NULL, &n->skip_validation, NULL, yyscanner); n->initially_valid = !n->skip_validation; + n->was_not_valid = n->skip_validation; $$ = (Node *) n; } ; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 23c9e3c5abf..a1878e565ff 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2819,6 +2819,7 @@ typedef struct Constraint bool is_enforced; /* enforced constraint? */ bool skip_validation; /* skip validation of existing rows? */ bool initially_valid; /* mark the new constraint as valid? */ + bool was_not_valid; /* did the user request NOT VALID? */ bool is_no_inherit; /* is constraint non-inheritable? */ Node *raw_expr; /* CHECK or DEFAULT expression, as * untransformed parse tree */ diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 362f38856d2..59736648ce4 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -574,6 +574,7 @@ DROP TABLE attmp2; -- exclusion until validated set constraint_exclusion TO 'partition'; create table nv_parent (d date, check (false) no inherit not valid); +WARNING: NOT VALID flag for constraint "nv_parent_check" ignored -- not valid constraint added at creation time should automatically become valid \d nv_parent Table "public.nv_parent" diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 6a3374d5152..b1e94a3d42b 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -61,7 +61,8 @@ DROP TABLE PKTABLE; -- CREATE TABLE PKTABLE ( ptest1 int, ptest2 int, ptest3 text, PRIMARY KEY(ptest1, ptest2) ); CREATE TABLE FKTABLE ( ftest1 int, ftest2 int, ftest3 int, CONSTRAINT constrname FOREIGN KEY(ftest1, ftest2) - REFERENCES PKTABLE MATCH FULL ON DELETE SET NULL ON UPDATE SET NULL); + REFERENCES PKTABLE MATCH FULL ON DELETE SET NULL ON UPDATE SET NULL NOT VALID); +WARNING: NOT VALID flag for constraint "constrname" ignored -- Test comments COMMENT ON CONSTRAINT constrname_wrong ON FKTABLE IS 'fk constraint comment'; ERROR: constraint "constrname_wrong" for table "fktable" does not exist diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 44945b0453a..83f1733253c 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -48,7 +48,7 @@ DROP TABLE PKTABLE; -- CREATE TABLE PKTABLE ( ptest1 int, ptest2 int, ptest3 text, PRIMARY KEY(ptest1, ptest2) ); CREATE TABLE FKTABLE ( ftest1 int, ftest2 int, ftest3 int, CONSTRAINT constrname FOREIGN KEY(ftest1, ftest2) - REFERENCES PKTABLE MATCH FULL ON DELETE SET NULL ON UPDATE SET NULL); + REFERENCES PKTABLE MATCH FULL ON DELETE SET NULL ON UPDATE SET NULL NOT VALID); -- Test comments COMMENT ON CONSTRAINT constrname_wrong ON FKTABLE IS 'fk constraint comment'; -- 2.39.5
