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

Commit56a047f

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 parentd491346 commit56a047f

File tree

5 files changed

+136
-10
lines changed

5 files changed

+136
-10
lines changed

‎src/backend/catalog/heap.c

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ static void StoreConstraints(Relation rel, List *cooked_constraints,
101101
boolis_internal);
102102
staticboolMergeWithExistingConstraint(Relationrel,char*ccname,Node*expr,
103103
boolallow_merge,boolis_local,
104+
boolis_initially_valid,
104105
boolis_no_inherit);
105106
staticvoidSetRelationNumChecks(Relationrel,intnumchecks);
106107
staticNode*cookConstraint(ParseState*pstate,
@@ -2262,6 +2263,7 @@ AddRelationNewConstraints(Relation rel,
22622263
*/
22632264
if (MergeWithExistingConstraint(rel,ccname,expr,
22642265
allow_merge,is_local,
2266+
cdef->initially_valid,
22652267
cdef->is_no_inherit))
22662268
continue;
22672269
}
@@ -2350,6 +2352,7 @@ AddRelationNewConstraints(Relation rel,
23502352
staticbool
23512353
MergeWithExistingConstraint(Relationrel,char*ccname,Node*expr,
23522354
boolallow_merge,boolis_local,
2355+
boolis_initially_valid,
23532356
boolis_no_inherit)
23542357
{
23552358
boolfound;
@@ -2397,22 +2400,47 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
23972400
if (equal(expr,stringToNode(TextDatumGetCString(val))))
23982401
found= true;
23992402
}
2403+
2404+
/*
2405+
* If the existing constraint is purely inherited (no local
2406+
* definition) then interpret addition of a local constraint as a
2407+
* legal merge. This allows ALTER ADD CONSTRAINT on parent and
2408+
* child tables to be given in either order with same end state.
2409+
*/
2410+
if (is_local&& !con->conislocal)
2411+
allow_merge= true;
2412+
24002413
if (!found|| !allow_merge)
24012414
ereport(ERROR,
24022415
(errcode(ERRCODE_DUPLICATE_OBJECT),
24032416
errmsg("constraint \"%s\" for relation \"%s\" already exists",
24042417
ccname,RelationGetRelationName(rel))));
24052418

2406-
tup=heap_copytuple(tup);
2407-
con= (Form_pg_constraint)GETSTRUCT(tup);
2408-
2409-
/* If the constraint is "no inherit" then cannot merge */
2419+
/* If the child constraint is "no inherit" then cannot merge */
24102420
if (con->connoinherit)
24112421
ereport(ERROR,
24122422
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
24132423
errmsg("constraint \"%s\" conflicts with non-inherited constraint on relation \"%s\"",
24142424
ccname,RelationGetRelationName(rel))));
24152425

2426+
/*
2427+
* If the child constraint is "not valid" then cannot merge with a
2428+
* valid parent constraint
2429+
*/
2430+
if (is_initially_valid&& !con->convalidated)
2431+
ereport(ERROR,
2432+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
2433+
errmsg("constraint \"%s\" conflicts with NOT VALID constraint on relation \"%s\"",
2434+
ccname,RelationGetRelationName(rel))));
2435+
2436+
/* OK to update the tuple */
2437+
ereport(NOTICE,
2438+
(errmsg("merging constraint \"%s\" with inherited definition",
2439+
ccname)));
2440+
2441+
tup=heap_copytuple(tup);
2442+
con= (Form_pg_constraint)GETSTRUCT(tup);
2443+
24162444
if (is_local)
24172445
con->conislocal= true;
24182446
else
@@ -2422,10 +2450,6 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
24222450
Assert(is_local);
24232451
con->connoinherit= true;
24242452
}
2425-
/* OK to update the tuple */
2426-
ereport(NOTICE,
2427-
(errmsg("merging constraint \"%s\" with inherited definition",
2428-
ccname)));
24292453
simple_heap_update(conDesc,&tup->t_self,tup);
24302454
CatalogUpdateIndexes(conDesc,tup);
24312455
break;

‎src/backend/commands/tablecmds.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9420,14 +9420,25 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
94209420
RelationGetRelationName(child_rel),
94219421
NameStr(parent_con->conname))));
94229422

9423-
/* If the constraint is "no inherit" then cannot merge */
9423+
/* If thechildconstraint is "no inherit" then cannot merge */
94249424
if (child_con->connoinherit)
94259425
ereport(ERROR,
94269426
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
94279427
errmsg("constraint \"%s\" conflicts with non-inherited constraint on child table \"%s\"",
94289428
NameStr(child_con->conname),
94299429
RelationGetRelationName(child_rel))));
94309430

9431+
/*
9432+
* If the child constraint is "not valid" then cannot merge with a
9433+
* valid parent constraint
9434+
*/
9435+
if (parent_con->convalidated&& !child_con->convalidated)
9436+
ereport(ERROR,
9437+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
9438+
errmsg("constraint \"%s\" conflicts with NOT VALID constraint on child table \"%s\"",
9439+
NameStr(child_con->conname),
9440+
RelationGetRelationName(child_rel))));
9441+
94319442
/*
94329443
* OK, bump the child constraint's inheritance count. (If we fail
94339444
* 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
@@ -1103,6 +1103,56 @@ Has OIDs: no
11031103
DROP TABLE test_foreign_constraints_inh;
11041104
DROP TABLE test_foreign_constraints;
11051105
DROP TABLE test_primary_constraints;
1106+
-- Test that parent and child CHECK constraints can be created in either order
1107+
create table p1(f1 int);
1108+
create table p1_c1() inherits(p1);
1109+
alter table p1 add constraint inh_check_constraint1 check (f1 > 0);
1110+
alter table p1_c1 add constraint inh_check_constraint1 check (f1 > 0);
1111+
NOTICE: merging constraint "inh_check_constraint1" with inherited definition
1112+
alter table p1_c1 add constraint inh_check_constraint2 check (f1 < 10);
1113+
alter table p1 add constraint inh_check_constraint2 check (f1 < 10);
1114+
NOTICE: merging constraint "inh_check_constraint2" with inherited definition
1115+
select conrelid::regclass::text as relname, conname, conislocal, coninhcount
1116+
from pg_constraint where conname like 'inh\_check\_constraint%'
1117+
order by 1, 2;
1118+
relname | conname | conislocal | coninhcount
1119+
---------+-----------------------+------------+-------------
1120+
p1 | inh_check_constraint1 | t | 0
1121+
p1 | inh_check_constraint2 | t | 0
1122+
p1_c1 | inh_check_constraint1 | t | 1
1123+
p1_c1 | inh_check_constraint2 | t | 1
1124+
(4 rows)
1125+
1126+
drop table p1 cascade;
1127+
NOTICE: drop cascades to table p1_c1
1128+
-- Test that a valid child can have not-valid parent, but not vice versa
1129+
create table invalid_check_con(f1 int);
1130+
create table invalid_check_con_child() inherits(invalid_check_con);
1131+
alter table invalid_check_con_child add constraint inh_check_constraint check(f1 > 0) not valid;
1132+
alter table invalid_check_con add constraint inh_check_constraint check(f1 > 0); -- fail
1133+
ERROR: constraint "inh_check_constraint" conflicts with NOT VALID constraint on relation "invalid_check_con_child"
1134+
alter table invalid_check_con_child drop constraint inh_check_constraint;
1135+
insert into invalid_check_con values(0);
1136+
alter table invalid_check_con_child add constraint inh_check_constraint check(f1 > 0);
1137+
alter table invalid_check_con add constraint inh_check_constraint check(f1 > 0) not valid;
1138+
NOTICE: merging constraint "inh_check_constraint" with inherited definition
1139+
insert into invalid_check_con values(0); -- fail
1140+
ERROR: new row for relation "invalid_check_con" violates check constraint "inh_check_constraint"
1141+
DETAIL: Failing row contains (0).
1142+
insert into invalid_check_con_child values(0); -- fail
1143+
ERROR: new row for relation "invalid_check_con_child" violates check constraint "inh_check_constraint"
1144+
DETAIL: Failing row contains (0).
1145+
select conrelid::regclass::text as relname, conname,
1146+
convalidated, conislocal, coninhcount, connoinherit
1147+
from pg_constraint where conname like 'inh\_check\_constraint%'
1148+
order by 1, 2;
1149+
relname | conname | convalidated | conislocal | coninhcount | connoinherit
1150+
-------------------------+----------------------+--------------+------------+-------------+--------------
1151+
invalid_check_con | inh_check_constraint | f | t | 0 | f
1152+
invalid_check_con_child | inh_check_constraint | t | t | 1 | f
1153+
(2 rows)
1154+
1155+
-- We don't drop the invalid_check_con* tables, to test dump/reload with
11061156
--
11071157
-- Test parameterized append plans for inheritance trees
11081158
--

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ SELECT relname, relhasindex
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
log_table | f
@@ -166,7 +168,7 @@ SELECT relname, relhasindex
166168
timetz_tbl | f
167169
tinterval_tbl | f
168170
varchar_tbl | f
169-
(155 rows)
171+
(157 rows)
170172

171173
--
172174
-- another sanity check: every system catalog that has OIDs should have

‎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