Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commite55a946

Browse files
committed
Fix two bugs in merging of inherited CHECK constraints.
Historically, we've allowed users to add a CHECK constraint to a childtable and then add an identical CHECK constraint to the parent. Thisresults in "merging" the two constraints so that the pre-existingchild constraint ends up with both conislocal = true and coninhcount > 0.However, if you tried to do it in the other order, you got a duplicateconstraint error. This is problematic for pg_dump, which needs to issueseparated ADD CONSTRAINT commands in some cases, but has no good way toensure that the constraints will be added in the required order.And it's more than a bit arbitrary, too. The goal of complaining aboutduplicated ADD CONSTRAINT commands can be served if we reject the case ofadding a constraint when the existing one already has conislocal = true;but if it has conislocal = false, let's just make the ADD CONSTRAINT setconislocal = true. In this way, either order of adding the constraintshas the same end result.Another problem was that the code allowed creation of a parent constraintmarked convalidated that is merged with a child constraint that is!convalidated. In this case, an inheritance scan of the parent table couldemit some rows violating the constraint condition, which would be anunexpected result given the marking of the parent constraint as validated.Hence, forbid merging of constraints in this case. (Note: valid child andnot-valid parent seems fine, so continue to allow that.)Per report from Benedikt Grundmann. Back-patch to 9.2 where we introducedpossibly-not-valid check constraints. The second bug obviously doesn'tapply before that, and I think the first doesn't either, because pg_dumponly gets into this situation when dealing with not-valid constraints.Report: <CADbMkNPT-Jz5PRSQ4RbUASYAjocV_KHUWapR%2Bg8fNvhUAyRpxA%40mail.gmail.com>Discussion: <22108.1475874586@sss.pgh.pa.us>
1 parent8811f5d commite55a946

File tree

5 files changed

+135
-9
lines changed

5 files changed

+135
-9
lines changed

‎src/backend/catalog/heap.c

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ static void StoreConstraints(Relation rel, List *cooked_constraints,
105105
boolis_internal);
106106
staticboolMergeWithExistingConstraint(Relationrel,char*ccname,Node*expr,
107107
boolallow_merge,boolis_local,
108+
boolis_initially_valid,
108109
boolis_no_inherit);
109110
staticvoidSetRelationNumChecks(Relationrel,intnumchecks);
110111
staticNode*cookConstraint(ParseState*pstate,
@@ -2301,6 +2302,7 @@ AddRelationNewConstraints(Relation rel,
23012302
*/
23022303
if (MergeWithExistingConstraint(rel,ccname,expr,
23032304
allow_merge,is_local,
2305+
cdef->initially_valid,
23042306
cdef->is_no_inherit))
23052307
continue;
23062308
}
@@ -2389,6 +2391,7 @@ AddRelationNewConstraints(Relation rel,
23892391
staticbool
23902392
MergeWithExistingConstraint(Relationrel,char*ccname,Node*expr,
23912393
boolallow_merge,boolis_local,
2394+
boolis_initially_valid,
23922395
boolis_no_inherit)
23932396
{
23942397
boolfound;
@@ -2436,22 +2439,47 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
24362439
if (equal(expr,stringToNode(TextDatumGetCString(val))))
24372440
found= true;
24382441
}
2442+
2443+
/*
2444+
* If the existing constraint is purely inherited (no local
2445+
* definition) then interpret addition of a local constraint as a
2446+
* legal merge. This allows ALTER ADD CONSTRAINT on parent and
2447+
* child tables to be given in either order with same end state.
2448+
*/
2449+
if (is_local&& !con->conislocal)
2450+
allow_merge= true;
2451+
24392452
if (!found|| !allow_merge)
24402453
ereport(ERROR,
24412454
(errcode(ERRCODE_DUPLICATE_OBJECT),
24422455
errmsg("constraint \"%s\" for relation \"%s\" already exists",
24432456
ccname,RelationGetRelationName(rel))));
24442457

2445-
tup=heap_copytuple(tup);
2446-
con= (Form_pg_constraint)GETSTRUCT(tup);
2447-
2448-
/* If the constraint is "no inherit" then cannot merge */
2458+
/* If the child constraint is "no inherit" then cannot merge */
24492459
if (con->connoinherit)
24502460
ereport(ERROR,
24512461
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
24522462
errmsg("constraint \"%s\" conflicts with non-inherited constraint on relation \"%s\"",
24532463
ccname,RelationGetRelationName(rel))));
24542464

2465+
/*
2466+
* If the child constraint is "not valid" then cannot merge with a
2467+
* valid parent constraint
2468+
*/
2469+
if (is_initially_valid&& !con->convalidated)
2470+
ereport(ERROR,
2471+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
2472+
errmsg("constraint \"%s\" conflicts with NOT VALID constraint on relation \"%s\"",
2473+
ccname,RelationGetRelationName(rel))));
2474+
2475+
/* OK to update the tuple */
2476+
ereport(NOTICE,
2477+
(errmsg("merging constraint \"%s\" with inherited definition",
2478+
ccname)));
2479+
2480+
tup=heap_copytuple(tup);
2481+
con= (Form_pg_constraint)GETSTRUCT(tup);
2482+
24552483
if (is_local)
24562484
con->conislocal= true;
24572485
else
@@ -2461,10 +2489,6 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
24612489
Assert(is_local);
24622490
con->connoinherit= true;
24632491
}
2464-
/* OK to update the tuple */
2465-
ereport(NOTICE,
2466-
(errmsg("merging constraint \"%s\" with inherited definition",
2467-
ccname)));
24682492
simple_heap_update(conDesc,&tup->t_self,tup);
24692493
CatalogUpdateIndexes(conDesc,tup);
24702494
break;

‎src/backend/commands/tablecmds.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10373,14 +10373,25 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
1037310373
RelationGetRelationName(child_rel),
1037410374
NameStr(parent_con->conname))));
1037510375

10376-
/* If the constraint is "no inherit" then cannot merge */
10376+
/* If thechildconstraint is "no inherit" then cannot merge */
1037710377
if (child_con->connoinherit)
1037810378
ereport(ERROR,
1037910379
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
1038010380
errmsg("constraint \"%s\" conflicts with non-inherited constraint on child table \"%s\"",
1038110381
NameStr(child_con->conname),
1038210382
RelationGetRelationName(child_rel))));
1038310383

10384+
/*
10385+
* If the child constraint is "not valid" then cannot merge with a
10386+
* valid parent constraint
10387+
*/
10388+
if (parent_con->convalidated&& !child_con->convalidated)
10389+
ereport(ERROR,
10390+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
10391+
errmsg("constraint \"%s\" conflicts with NOT VALID constraint on child table \"%s\"",
10392+
NameStr(child_con->conname),
10393+
RelationGetRelationName(child_rel))));
10394+
1038410395
/*
1038510396
* OK, bump the child constraint's inheritance count. (If we fail
1038610397
* later on, this change will just roll back.)

‎src/test/regress/expected/inherit.out

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,6 +1090,56 @@ Inherits: test_foreign_constraints
10901090
DROP TABLE test_foreign_constraints_inh;
10911091
DROP TABLE test_foreign_constraints;
10921092
DROP TABLE test_primary_constraints;
1093+
-- Test that parent and child CHECK constraints can be created in either order
1094+
create table p1(f1 int);
1095+
create table p1_c1() inherits(p1);
1096+
alter table p1 add constraint inh_check_constraint1 check (f1 > 0);
1097+
alter table p1_c1 add constraint inh_check_constraint1 check (f1 > 0);
1098+
NOTICE: merging constraint "inh_check_constraint1" with inherited definition
1099+
alter table p1_c1 add constraint inh_check_constraint2 check (f1 < 10);
1100+
alter table p1 add constraint inh_check_constraint2 check (f1 < 10);
1101+
NOTICE: merging constraint "inh_check_constraint2" with inherited definition
1102+
select conrelid::regclass::text as relname, conname, conislocal, coninhcount
1103+
from pg_constraint where conname like 'inh\_check\_constraint%'
1104+
order by 1, 2;
1105+
relname | conname | conislocal | coninhcount
1106+
---------+-----------------------+------------+-------------
1107+
p1 | inh_check_constraint1 | t | 0
1108+
p1 | inh_check_constraint2 | t | 0
1109+
p1_c1 | inh_check_constraint1 | t | 1
1110+
p1_c1 | inh_check_constraint2 | t | 1
1111+
(4 rows)
1112+
1113+
drop table p1 cascade;
1114+
NOTICE: drop cascades to table p1_c1
1115+
-- Test that a valid child can have not-valid parent, but not vice versa
1116+
create table invalid_check_con(f1 int);
1117+
create table invalid_check_con_child() inherits(invalid_check_con);
1118+
alter table invalid_check_con_child add constraint inh_check_constraint check(f1 > 0) not valid;
1119+
alter table invalid_check_con add constraint inh_check_constraint check(f1 > 0); -- fail
1120+
ERROR: constraint "inh_check_constraint" conflicts with NOT VALID constraint on relation "invalid_check_con_child"
1121+
alter table invalid_check_con_child drop constraint inh_check_constraint;
1122+
insert into invalid_check_con values(0);
1123+
alter table invalid_check_con_child add constraint inh_check_constraint check(f1 > 0);
1124+
alter table invalid_check_con add constraint inh_check_constraint check(f1 > 0) not valid;
1125+
NOTICE: merging constraint "inh_check_constraint" with inherited definition
1126+
insert into invalid_check_con values(0); -- fail
1127+
ERROR: new row for relation "invalid_check_con" violates check constraint "inh_check_constraint"
1128+
DETAIL: Failing row contains (0).
1129+
insert into invalid_check_con_child values(0); -- fail
1130+
ERROR: new row for relation "invalid_check_con_child" violates check constraint "inh_check_constraint"
1131+
DETAIL: Failing row contains (0).
1132+
select conrelid::regclass::text as relname, conname,
1133+
convalidated, conislocal, coninhcount, connoinherit
1134+
from pg_constraint where conname like 'inh\_check\_constraint%'
1135+
order by 1, 2;
1136+
relname | conname | convalidated | conislocal | coninhcount | connoinherit
1137+
-------------------------+----------------------+--------------+------------+-------------+--------------
1138+
invalid_check_con | inh_check_constraint | f | t | 0 | f
1139+
invalid_check_con_child | inh_check_constraint | t | t | 1 | f
1140+
(2 rows)
1141+
1142+
-- We don't drop the invalid_check_con* tables, to test dump/reload with
10931143
--
10941144
-- Test parameterized append plans for inheritance trees
10951145
--

‎src/test/regress/expected/sanity_check.out

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ int2_tbl|f
6262
int4_tbl|f
6363
int8_tbl|f
6464
interval_tbl|f
65+
invalid_check_con|f
66+
invalid_check_con_child|f
6567
iportaltest|f
6668
kd_point_tbl|t
6769
line_tbl|f

‎src/test/regress/sql/inherit.sql

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,45 @@ DROP TABLE test_foreign_constraints_inh;
334334
DROPTABLE test_foreign_constraints;
335335
DROPTABLE test_primary_constraints;
336336

337+
-- Test that parent and child CHECK constraints can be created in either order
338+
createtablep1(f1int);
339+
createtablep1_c1() inherits(p1);
340+
341+
altertable p1 addconstraint inh_check_constraint1check (f1>0);
342+
altertable p1_c1 addconstraint inh_check_constraint1check (f1>0);
343+
344+
altertable p1_c1 addconstraint inh_check_constraint2check (f1<10);
345+
altertable p1 addconstraint inh_check_constraint2check (f1<10);
346+
347+
select conrelid::regclass::textas relname, conname, conislocal, coninhcount
348+
from pg_constraintwhere connamelike'inh\_check\_constraint%'
349+
order by1,2;
350+
351+
droptable p1 cascade;
352+
353+
-- Test that a valid child can have not-valid parent, but not vice versa
354+
createtableinvalid_check_con(f1int);
355+
createtableinvalid_check_con_child() inherits(invalid_check_con);
356+
357+
altertable invalid_check_con_child addconstraint inh_check_constraintcheck(f1>0) not valid;
358+
altertable invalid_check_con addconstraint inh_check_constraintcheck(f1>0);-- fail
359+
altertable invalid_check_con_child dropconstraint inh_check_constraint;
360+
361+
insert into invalid_check_convalues(0);
362+
363+
altertable invalid_check_con_child addconstraint inh_check_constraintcheck(f1>0);
364+
altertable invalid_check_con addconstraint inh_check_constraintcheck(f1>0) not valid;
365+
366+
insert into invalid_check_convalues(0);-- fail
367+
insert into invalid_check_con_childvalues(0);-- fail
368+
369+
select conrelid::regclass::textas relname, conname,
370+
convalidated, conislocal, coninhcount, connoinherit
371+
from pg_constraintwhere connamelike'inh\_check\_constraint%'
372+
order by1,2;
373+
374+
-- We don't drop the invalid_check_con* tables, to test dump/reload with
375+
337376
--
338377
-- Test parameterized append plans for inheritance trees
339378
--

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp