On 2015/12/09 11:19, Amit Langote wrote:
> On 2015/12/09 5:50, Robert Haas wrote:
>> I suspect this is an oversight. We allowed FOREIGN KEY constraints to
>> be not valid in 722bf7017bbe796decc79c1fde03e7a83dae9ada by Simon
>> Riggs, but didn't add allow it for CHECK constraints until Alvaro's
>> commit of 897795240cfaaed724af2f53ed2c50c9862f951f a few months later.
>> My guess is that there's no reason for these not to behave in the same
>> way, but they don't. Amul's proposed one-liner might be one part of
>> actually fixing that, but it wouldn't be enough by itself: you'd also
>> need to teach transformCreateStmt to set the initially_valid flag to
>> true, maybe by adding a new function transformCheckConstraints or so.
>
> So, any NOT VALID specification for a FK constraint is effectively
> overridden in transformFKConstraints() at table creation time but the same
> doesn't happen for CHECK constraints. I agree that that could be fixed,
> then as you say, Amul's one-liner would make sense.
So, how about attached?
I think it may be enough to flip initially_valid to true in
transformTableConstraint() when in a CREATE TABLE context.
Regarding Amul's proposed change, there arises one minor inconsistency.
StoreRelCheck() is called in two places - AddRelationNewConstraints(),
where we can safely change from passing the value of !skip_validation to
that of initially_valid and StoreConstraints(), where we cannot because
CookedConstraint is used which doesn't have the initially_valid field.
Nevertheless, attached patch includes the former.
Thoughts?
Thanks,
Amit
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 7d7d062..04c4f8f 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2349,7 +2349,7 @@ AddRelationNewConstraints(Relation rel,
* OK, store it.
*/
constrOid =
- StoreRelCheck(rel, ccname, expr, !cdef->skip_validation, is_local,
+ StoreRelCheck(rel, ccname, expr, cdef->initially_valid, is_local,
is_local ? 0 : 1, cdef->is_no_inherit, is_internal);
numchecks++;
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 344a40c..ce45804 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -687,6 +687,10 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
break;
case CONSTR_CHECK:
+ /* Is this better done in a transformCheckConstraint? */
+ if (!cxt->isalter)
+ constraint->initially_valid = true;
+
cxt->ckconstraints = lappend(cxt->ckconstraints, constraint);
break;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 61669b6..c91342f 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -187,7 +187,7 @@ Table "public.constraint_rename_test"
b | integer |
c | integer |
Check constraints:
- "con1" CHECK (a > 0)
+ "con1" CHECK (a > 0) NOT VALID
CREATE TABLE constraint_rename_test2 (a int CONSTRAINT con1 CHECK (a > 0), d int) INHERITS (constraint_rename_test);
NOTICE: merging column "a" with inherited definition
@@ -217,7 +217,7 @@ Table "public.constraint_rename_test"
b | integer |
c | integer |
Check constraints:
- "con1foo" CHECK (a > 0)
+ "con1foo" CHECK (a > 0) NOT VALID
Number of child tables: 1 (Use \d+ to list them.)
\d constraint_rename_test2
@@ -243,7 +243,7 @@ Table "public.constraint_rename_test"
b | integer |
c | integer |
Check constraints:
- "con1foo" CHECK (a > 0)
+ "con1foo" CHECK (a > 0) NOT VALID
"con2bar" CHECK (b > 0) NO INHERIT
Number of child tables: 1 (Use \d+ to list them.)
@@ -271,7 +271,7 @@ Table "public.constraint_rename_test"
Indexes:
"con3foo" PRIMARY KEY, btree (a)
Check constraints:
- "con1foo" CHECK (a > 0)
+ "con1foo" CHECK (a > 0) NOT VALID
"con2bar" CHECK (b > 0) NO INHERIT
Number of child tables: 1 (Use \d+ to list them.)
@@ -1820,7 +1820,7 @@ CREATE TABLE test_inh_check_child() INHERITS(test_inh_check);
a | double precision |
b | double precision |
Check constraints:
- "test_inh_check_a_check" CHECK (a > 10.2::double precision)
+ "test_inh_check_a_check" CHECK (a > 10.2::double precision) NOT VALID
Number of child tables: 1 (Use \d+ to list them.)
\d test_inh_check_child
@@ -1851,7 +1851,7 @@ ALTER TABLE test_inh_check ALTER COLUMN a TYPE numeric;
a | numeric |
b | double precision |
Check constraints:
- "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
+ "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) NOT VALID
Number of child tables: 1 (Use \d+ to list them.)
\d test_inh_check_child
@@ -1861,7 +1861,7 @@ Number of child tables: 1 (Use \d+ to list them.)
a | numeric |
b | double precision |
Check constraints:
- "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
+ "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) NOT VALID
Inherits: test_inh_check
select relname, conname, coninhcount, conislocal, connoinherit
@@ -1889,7 +1889,7 @@ NOTICE: merging constraint "bmerged" with inherited definition
Check constraints:
"bmerged" CHECK (b > 1::double precision)
"bnoinherit" CHECK (b > 100::double precision) NO INHERIT
- "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
+ "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) NOT VALID
Number of child tables: 1 (Use \d+ to list them.)
\d test_inh_check_child
@@ -1901,7 +1901,7 @@ Number of child tables: 1 (Use \d+ to list them.)
Check constraints:
"blocal" CHECK (b < 1000::double precision)
"bmerged" CHECK (b > 1::double precision)
- "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
+ "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) NOT VALID
Inherits: test_inh_check
select relname, conname, coninhcount, conislocal, connoinherit
@@ -1929,7 +1929,7 @@ Table "public.test_inh_check"
Check constraints:
"bmerged" CHECK (b::double precision > 1::double precision)
"bnoinherit" CHECK (b::double precision > 100::double precision) NO INHERIT
- "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
+ "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) NOT VALID
Number of child tables: 1 (Use \d+ to list them.)
\d test_inh_check_child
@@ -1941,7 +1941,7 @@ Table "public.test_inh_check_child"
Check constraints:
"blocal" CHECK (b::double precision < 1000::double precision)
"bmerged" CHECK (b::double precision > 1::double precision)
- "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision)
+ "test_inh_check_a_check" CHECK (a::double precision > 10.2::double precision) NOT VALID
Inherits: test_inh_check
select relname, conname, coninhcount, conislocal, connoinherit
diff --git a/src/test/regress/expected/create_table_like.out b/src/test/regress/expected/create_table_like.out
index 97edde1..55b3706 100644
--- a/src/test/regress/expected/create_table_like.out
+++ b/src/test/regress/expected/create_table_like.out
@@ -171,7 +171,7 @@ NOTICE: merging column "a" with inherited definition
c | text | | external | | C
Check constraints:
"ctlt1_a_check" CHECK (length(a) > 2)
- "ctlt3_a_check" CHECK (length(a) < 5)
+ "ctlt3_a_check" CHECK (length(a) < 5) NOT VALID
Inherits: ctlt1
SELECT description FROM pg_description, pg_constraint c WHERE classoid = 'pg_constraint'::regclass AND objoid = c.oid AND c.conrelid = 'ctlt13_like'::regclass;
@@ -192,7 +192,7 @@ Indexes:
"ctlt_all_b_idx" btree (b)
"ctlt_all_expr_idx" btree ((a || b))
Check constraints:
- "ctlt1_a_check" CHECK (length(a) > 2)
+ "ctlt1_a_check" CHECK (length(a) > 2) NOT VALID
SELECT c.relname, objsubid, description FROM pg_description, pg_index i, pg_class c WHERE classoid = 'pg_class'::regclass AND objoid = i.indexrelid AND c.oid = i.indexrelid AND i.indrelid = 'ctlt_all'::regclass ORDER BY c.relname, objsubid;
relname | objsubid | description
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 73c02bb..4596dbc 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -709,7 +709,7 @@ COMMENT ON COLUMN ft1.c1 IS 'ft1.c1';
c2 | text | | (param2 'val2', param3 'val3') | extended | |
c3 | date | | | plain | |
Check constraints:
- "ft1_c2_check" CHECK (c2 <> ''::text)
+ "ft1_c2_check" CHECK (c2 <> ''::text) NOT VALID
"ft1_c3_check" CHECK (c3 >= '01-01-1994'::date AND c3 <= '01-31-1994'::date)
Server: s0
FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
@@ -771,7 +771,7 @@ ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 SET STORAGE PLAIN;
c9 | integer | | | plain | |
c10 | integer | | (p1 'v1') | plain | |
Check constraints:
- "ft1_c2_check" CHECK (c2 <> ''::text)
+ "ft1_c2_check" CHECK (c2 <> ''::text) NOT VALID
"ft1_c3_check" CHECK (c3 >= '01-01-1994'::date AND c3 <= '01-31-1994'::date)
Server: s0
FDW Options: (delimiter ',', quote '"', "be quoted" 'value')
@@ -820,7 +820,7 @@ ALTER FOREIGN TABLE foreign_schema.ft1 RENAME TO foreign_table_1;
c8 | text | | (p2 'V2')
c10 | integer | | (p1 'v1')
Check constraints:
- "ft1_c2_check" CHECK (c2 <> ''::text)
+ "ft1_c2_check" CHECK (c2 <> ''::text) NOT VALID
"ft1_c3_check" CHECK (c3 >= '01-01-1994'::date AND c3 <= '01-31-1994'::date)
Server: s0
FDW Options: (quote '~', "be quoted" 'value', escape '@')
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index 89b6c1c..7853e41 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -849,7 +849,7 @@ alter table pp1 add column a1 int check (a1 > 0);
f3 | integer |
a1 | integer |
Check constraints:
- "pp1_a1_check" CHECK (a1 > 0)
+ "pp1_a1_check" CHECK (a1 > 0) NOT VALID
Inherits: pp1
create table cc2(f4 float) inherits(pp1,cc1);
@@ -884,7 +884,7 @@ NOTICE: merging constraint "pp1_a2_check" with inherited definition
a2 | integer |
Check constraints:
"pp1_a1_check" CHECK (a1 > 0)
- "pp1_a2_check" CHECK (a2 > 0)
+ "pp1_a2_check" CHECK (a2 > 0) NOT VALID
Inherits: pp1,
cc1
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
