public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alvaro Herrera <[email protected]>
To: Yasuo Honda <[email protected]>
Cc: jian he <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: CREATE TABLE NOT VALID for check and foreign key
Date: Tue, 14 Jan 2025 18:46:59 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
On 2025-Jan-08, Alvaro Herrera wrote:
> On 2025-Jan-07, Yasuo Honda wrote:
>
> > I'd like PostgreSQL to raise errors and/or warnings for the NOT VALID
> > check constraint for CREATE TABLE.
> > Ruby on Rails supports creating check constraints with the NOT VALID
> > option and I was not aware that it is just ignored until
> > https://github.com/rails/rails/issues/53732 issue is reported.
>
> Thanks. I left a comment there. I think raising a WARNING might work.
I think it'd be something like the rough POC here. I didn't add
warnings to all the CreateConstraintEntry() because some of them aren't
reachable via a path that allows NOT VALID. However, I may have missed
some cases.
I was really surprised to find that we have no test coverage for the
case where CREATE TABLE specifies a NOT VALID constraint.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Attachments:
[text/x-diff] 0001-Throw-warning-if-NOT-VALID-is-given-during-CREATE-TA.patch (5.8K, ../[email protected]/2-0001-Throw-warning-if-NOT-VALID-is-given-during-CREATE-TA.patch)
download | inline diff:
From 08dd8b9a65223dfb3662a509b9da54fcb8adb520 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]>
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/CACJufxEQcHNhN6M18JY1mQcgQq9Gn9ofMeop47SdFDE5B8wbug@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 57ef466acce..8a39dc1c2da 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2510,6 +2510,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.
*/
@@ -2967,6 +2973,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 f070ffc960e..8e498344e7a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10446,6 +10446,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 6079de70e09..c008a445bd4 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4172,6 +4172,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
@@ -4306,6 +4307,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 b191eaaecab..2f103238d8f 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2762,6 +2762,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 dd8cdec2905..8b5918b9eb6 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 3f459f70ac1..3b528738302 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 2e710e419c2..46a043e18e1 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
view thread (3+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: CREATE TABLE NOT VALID for check and foreign key
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox