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

Commitd9f686a

Browse files
committed
Fix restore of not-null constraints with inheritance
In tables with primary keys, pg_dump creates tables with primary keys byinitially dumping them with throw-away not-null constraints (marked "noinherit" so that they don't create problems elsewhere), to later dropthem once the primary key is restored. Because of a unrelatedconsideration, on tables with children we add not-null constraints toall columns of the primary key when it is created.If both a table and its child have primary keys, and pg_dump happens toemit the child table first (and its throw-away not-null) and later itsparent table, the creation of the parent's PK will fail because thethrow-away not-null constraint collides with the permanent not-nullconstraint that the PK wants to add, so the dump fails to restore.We can work around this problem by letting the primary key "take over"the child's not-null. This requires no changes to pg_dump, just twochanges to ALTER TABLE: first, the ability to convert a no-inheritnot-null constraint into a regular inheritable one (including recursingdown to children, if there are any); second, the ability to "drop" aconstraint that is defined both directly in the table and inherited froma parent (which simply means to mark it as no longer having a localdefinition).Secondarily, change ATPrepAddPrimaryKey() to acquire locks all the waydown the inheritance hierarchy, in case we need to recurse whenpropagating constraints.These two changes allow pg_dump to reproduce more cases involvinginheritance from versions 16 and older.Lastly, make two changes to pg_dump: 1) do not try to drop a not-nullconstraint that's marked as inherited; this allows a dump to restorewith no errors if a table with a PK inherits from another which also hasa PK; 2) avoid giving inherited constraints throwaway names, for therare cases where such a constraint survives after the restore.Reported-by: Andrew Bille <andrewbille@gmail.com>Reported-by: Justin Pryzby <pryzby@telsasoft.com>Discussion:https://postgr.es/m/CAJnzarwkfRu76_yi3dqVF_WL-MpvT54zMwAxFwJceXdHB76bOA@mail.gmail.comDiscussion:https://postgr.es/m/Zh0aAH7tbZb-9HbC@pryzbyj2023
1 parente0d51e3 commitd9f686a

File tree

7 files changed

+221
-29
lines changed

7 files changed

+221
-29
lines changed

‎src/backend/catalog/heap.c

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2539,6 +2539,7 @@ AddRelationNewConstraints(Relation rel,
25392539
CookedConstraint*nncooked;
25402540
AttrNumbercolnum;
25412541
char*nnname;
2542+
intexisting;
25422543

25432544
/* Determine which column to modify */
25442545
colnum=get_attnum(RelationGetRelid(rel),strVal(linitial(cdef->keys)));
@@ -2547,12 +2548,39 @@ AddRelationNewConstraints(Relation rel,
25472548
strVal(linitial(cdef->keys)),RelationGetRelid(rel));
25482549

25492550
/*
2550-
* If the column already has a not-null constraint, we need only
2551-
* update its catalog status and we're done.
2551+
* If the column already has an inheritable not-null constraint,
2552+
* we need only adjust its inheritance status and we're done. If
2553+
* the constraint is there but marked NO INHERIT, then it is
2554+
* updated in the same way, but we also recurse to the children
2555+
* (if any) to add the constraint there as well.
25522556
*/
2553-
if (AdjustNotNullInheritance1(RelationGetRelid(rel),colnum,
2554-
cdef->inhcount,cdef->is_no_inherit))
2557+
existing=AdjustNotNullInheritance1(RelationGetRelid(rel),colnum,
2558+
cdef->inhcount,cdef->is_no_inherit);
2559+
if (existing==1)
2560+
continue;/* all done! */
2561+
elseif (existing==-1)
2562+
{
2563+
List*children;
2564+
2565+
children=find_inheritance_children(RelationGetRelid(rel),NoLock);
2566+
foreach_oid(childoid,children)
2567+
{
2568+
Relationchildrel=table_open(childoid,NoLock);
2569+
2570+
AddRelationNewConstraints(childrel,
2571+
NIL,
2572+
list_make1(copyObject(cdef)),
2573+
allow_merge,
2574+
is_local,
2575+
is_internal,
2576+
queryString);
2577+
/* these constraints are not in the return list -- good? */
2578+
2579+
table_close(childrel,NoLock);
2580+
}
2581+
25552582
continue;
2583+
}
25562584

25572585
/*
25582586
* If a constraint name is specified, check that it isn't already

‎src/backend/catalog/pg_constraint.c

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -565,8 +565,8 @@ ChooseConstraintName(const char *name1, const char *name2,
565565
}
566566

567567
/*
568-
* Find and return the pg_constraint tuple that implements a validated
569-
* not-null constraint for the given column of the given relation.
568+
* Find and returna copy ofthe pg_constraint tuple that implements a
569+
*validatednot-null constraint for the given column of the given relation.
570570
*
571571
* XXX This would be easier if we had pg_attribute.notnullconstr with the OID
572572
* of the constraint that implements the not-null constraint for that column.
@@ -709,37 +709,54 @@ extractNotNullColumn(HeapTuple constrTup)
709709
* AdjustNotNullInheritance1
710710
*Adjust inheritance count for a single not-null constraint
711711
*
712-
* Adjust inheritance count, and possibly islocal status, for the not-null
713-
* constraint row of the given column, if it exists, and return true.
714-
* If no not-null constraint is found for the column, return false.
712+
* If no not-null constraint is found for the column, return 0.
713+
* Caller can create one.
714+
* If the constraint does exist and it's inheritable, adjust its
715+
* inheritance count (and possibly islocal status) and return 1.
716+
* No further action needs to be taken.
717+
* If the constraint exists but is marked NO INHERIT, adjust it as above
718+
* and reset connoinherit to false, and return -1. Caller is
719+
* responsible for adding the same constraint to the children, if any.
715720
*/
716-
bool
721+
int
717722
AdjustNotNullInheritance1(Oidrelid,AttrNumberattnum,intcount,
718723
boolis_no_inherit)
719724
{
720725
HeapTupletup;
721726

727+
Assert(count >=0);
728+
722729
tup=findNotNullConstraintAttnum(relid,attnum);
723730
if (HeapTupleIsValid(tup))
724731
{
725732
Relationpg_constraint;
726733
Form_pg_constraintconform;
734+
intretval=1;
727735

728736
pg_constraint=table_open(ConstraintRelationId,RowExclusiveLock);
729737
conform= (Form_pg_constraint)GETSTRUCT(tup);
730738

731739
/*
732-
* Don't let the NO INHERIT status change (but don't complain
733-
* unnecessarily.) In the future it might be useful to let an
734-
* inheritable constraint replace a non-inheritable one, but we'd need
735-
* to recurse to children to get it added there.
740+
* If we're asked for a NO INHERIT constraint and this relation
741+
* already has an inheritable one, throw an error.
736742
*/
737-
if (is_no_inherit!=conform->connoinherit)
743+
if (is_no_inherit&& !conform->connoinherit)
738744
ereport(ERROR,
739745
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
740746
errmsg("cannot change NO INHERIT status of inherited NOT NULL constraint \"%s\" on relation \"%s\"",
741747
NameStr(conform->conname),get_rel_name(relid)));
742748

749+
/*
750+
* If the constraint already exists in this relation but it's marked
751+
* NO INHERIT, we can just remove that flag, and instruct caller to
752+
* recurse to add the constraint to children.
753+
*/
754+
if (!is_no_inherit&&conform->connoinherit)
755+
{
756+
conform->connoinherit= false;
757+
retval=-1;/* caller must add constraint on child rels */
758+
}
759+
743760
if (count>0)
744761
conform->coninhcount+=count;
745762

@@ -761,10 +778,10 @@ AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count,
761778

762779
table_close(pg_constraint,RowExclusiveLock);
763780

764-
returntrue;
781+
returnretval;
765782
}
766783

767-
returnfalse;
784+
return0;
768785
}
769786

770787
/*

‎src/backend/commands/tablecmds.c

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9336,7 +9336,6 @@ ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd,
93369336
{
93379337
List *children;
93389338
List *newconstrs = NIL;
9339-
ListCell *lc;
93409339
IndexStmt *indexstmt;
93419340

93429341
/* No work if not creating a primary key */
@@ -9351,11 +9350,19 @@ ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd,
93519350
!rel->rd_rel->relhassubclass)
93529351
return;
93539352

9354-
children = find_inheritance_children(RelationGetRelid(rel), lockmode);
9353+
/*
9354+
* Acquire locks all the way down the hierarchy. The recursion to lower
9355+
* levels occurs at execution time as necessary, so we don't need to do it
9356+
* here, and we don't need the returned list either.
9357+
*/
9358+
(void) find_all_inheritors(RelationGetRelid(rel), lockmode, NULL);
93559359

9356-
foreach(lc, indexstmt->indexParams)
9360+
/*
9361+
* Construct the list of constraints that we need to add to each child
9362+
* relation.
9363+
*/
9364+
foreach_node(IndexElem, elem, indexstmt->indexParams)
93579365
{
9358-
IndexElem *elem = lfirst_node(IndexElem, lc);
93599366
Constraint *nnconstr;
93609367

93619368
Assert(elem->expr == NULL);
@@ -9374,9 +9381,10 @@ ATPrepAddPrimaryKey(List **wqueue, Relation rel, AlterTableCmd *cmd,
93749381
newconstrs = lappend(newconstrs, nnconstr);
93759382
}
93769383

9377-
foreach(lc, children)
9384+
/* Finally, add AT subcommands to add each constraint to each child. */
9385+
children = find_inheritance_children(RelationGetRelid(rel), NoLock);
9386+
foreach_oid(childrelid, children)
93789387
{
9379-
Oidchildrelid = lfirst_oid(lc);
93809388
Relationchildrel = table_open(childrelid, NoLock);
93819389
AlterTableCmd *newcmd = makeNode(AlterTableCmd);
93829390
ListCell *lc2;
@@ -12942,6 +12950,31 @@ dropconstraint_internal(Relation rel, HeapTuple constraintTup, DropBehavior beha
1294212950
con = (Form_pg_constraint) GETSTRUCT(constraintTup);
1294312951
constrName = NameStr(con->conname);
1294412952

12953+
/*
12954+
* If we're asked to drop a constraint which is both defined locally and
12955+
* inherited, we can simply mark it as no longer having a local
12956+
* definition, and no further changes are required.
12957+
*
12958+
* XXX We do this for not-null constraints only, not CHECK, because the
12959+
* latter have historically not behaved this way and it might be confusing
12960+
* to change the behavior now.
12961+
*/
12962+
if (con->contype == CONSTRAINT_NOTNULL &&
12963+
con->conislocal && con->coninhcount > 0)
12964+
{
12965+
HeapTuplecopytup;
12966+
12967+
copytup = heap_copytuple(constraintTup);
12968+
con = (Form_pg_constraint) GETSTRUCT(copytup);
12969+
con->conislocal = false;
12970+
CatalogTupleUpdate(conrel, &copytup->t_self, copytup);
12971+
ObjectAddressSet(conobj, ConstraintRelationId, con->oid);
12972+
12973+
CommandCounterIncrement();
12974+
table_close(conrel, RowExclusiveLock);
12975+
return conobj;
12976+
}
12977+
1294512978
/* Don't allow drop of inherited constraints */
1294612979
if (con->coninhcount > 0 && !recursing)
1294712980
ereport(ERROR,
@@ -16620,7 +16653,25 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
1662016653
errmsg("too many inheritance parents"));
1662116654
if (child_con->contype == CONSTRAINT_NOTNULL &&
1662216655
child_con->connoinherit)
16656+
{
16657+
/*
16658+
* If the child has children, it's not possible to turn a NO
16659+
* INHERIT constraint into an inheritable one: we would need
16660+
* to recurse to create constraints in those children, but
16661+
* this is not a good place to do that.
16662+
*/
16663+
if (child_rel->rd_rel->relhassubclass)
16664+
ereport(ERROR,
16665+
errmsg("cannot add NOT NULL constraint to column \"%s\" of relation \"%s\" with inheritance children",
16666+
get_attname(RelationGetRelid(child_rel),
16667+
extractNotNullColumn(child_tuple),
16668+
false),
16669+
RelationGetRelationName(child_rel)),
16670+
errdetail("Existing constraint \"%s\" is marked NO INHERIT.",
16671+
NameStr(child_con->conname)));
16672+
1662316673
child_con->connoinherit = false;
16674+
}
1662416675

1662516676
/*
1662616677
* In case of partitions, an inherited constraint must be
@@ -20225,7 +20276,7 @@ ATExecDetachPartitionFinalize(Relation rel, RangeVar *name)
2022520276
* DetachAddConstraintIfNeeded
2022620277
*Subroutine for ATExecDetachPartition. Create a constraint that
2022720278
*takes the place of the partition constraint, but avoid creating
20228-
*a dupe ifan constraint already exists which implies the needed
20279+
*a dupe ifa constraint already exists which implies the needed
2022920280
*constraint.
2023020281
*/
2023120282
static void

‎src/bin/pg_dump/pg_dump.c

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9096,8 +9096,21 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
90969096
}
90979097
else if (use_throwaway_notnull)
90989098
{
9099-
tbinfo->notnull_constrs[j] =
9100-
psprintf("pgdump_throwaway_notnull_%d", notnullcount++);
9099+
/*
9100+
* Decide on a name for this constraint. If it is not an
9101+
* inherited constraint, give it a throwaway name to avoid any
9102+
* possible conflicts, since we're going to drop it soon
9103+
* anyway. If it is inherited then try harder, because it may
9104+
* (but not necessarily) persist after the restore.
9105+
*/
9106+
if (tbinfo->notnull_inh[j])
9107+
/* XXX maybe try harder if the name is overlength */
9108+
tbinfo->notnull_constrs[j] =
9109+
psprintf("%s_%s_not_null",
9110+
tbinfo->dobj.name, tbinfo->attnames[j]);
9111+
else
9112+
tbinfo->notnull_constrs[j] =
9113+
psprintf("pgdump_throwaway_notnull_%d", notnullcount++);
91019114
tbinfo->notnull_throwaway[j] = true;
91029115
tbinfo->notnull_inh[j] = false;
91039116
}
@@ -17325,10 +17338,15 @@ dumpConstraint(Archive *fout, const ConstraintInfo *coninfo)
1732517338
* similar code in dumpIndex!
1732617339
*/
1732717340

17328-
/* Drop any not-null constraints that were added to support the PK */
17341+
/*
17342+
* Drop any not-null constraints that were added to support the PK,
17343+
* but leave them alone if they have a definition coming from their
17344+
* parent.
17345+
*/
1732917346
if (coninfo->contype == 'p')
1733017347
for (int i = 0; i < tbinfo->numatts; i++)
17331-
if (tbinfo->notnull_throwaway[i])
17348+
if (tbinfo->notnull_throwaway[i] &&
17349+
!tbinfo->notnull_inh[i])
1733217350
appendPQExpBuffer(q, "\nALTER TABLE ONLY %s DROP CONSTRAINT %s;",
1733317351
fmtQualifiedDumpable(tbinfo),
1733417352
tbinfo->notnull_constrs[i]);

‎src/include/catalog/pg_constraint.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ extern HeapTuple findNotNullConstraintAttnum(Oid relid, AttrNumber attnum);
261261
externHeapTuplefindNotNullConstraint(Oidrelid,constchar*colname);
262262
externHeapTuplefindDomainNotNullConstraint(Oidtypid);
263263
externAttrNumberextractNotNullColumn(HeapTupleconstrTup);
264-
externboolAdjustNotNullInheritance1(Oidrelid,AttrNumberattnum,intcount,
264+
externintAdjustNotNullInheritance1(Oidrelid,AttrNumberattnum,intcount,
265265
boolis_no_inherit);
266266
externvoidAdjustNotNullInheritance(Oidrelid,Bitmapset*columns,intcount);
267267
externList*RelationGetNotNullConstraints(Oidrelid,boolcooked);

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,62 @@ ALTER TABLE ATACC1 ADD NOT NULL a NO INHERIT;
321321
Inherits: atacc1
322322

323323
DROP TABLE ATACC1, ATACC2;
324+
-- overridding a no-inherit constraint with an inheritable one
325+
CREATE TABLE ATACC2 (a int, CONSTRAINT a_is_not_null NOT NULL a NO INHERIT);
326+
CREATE TABLE ATACC1 (a int);
327+
CREATE TABLE ATACC3 (a int) INHERITS (ATACC2);
328+
NOTICE: merging column "a" with inherited definition
329+
INSERT INTO ATACC3 VALUES (null);-- make sure we scan atacc3
330+
ALTER TABLE ATACC2 INHERIT ATACC1;
331+
ALTER TABLE ATACC1 ADD CONSTRAINT ditto NOT NULL a;
332+
ERROR: column "a" of relation "atacc3" contains null values
333+
DELETE FROM ATACC3;
334+
ALTER TABLE ATACC1 ADD CONSTRAINT ditto NOT NULL a;
335+
\d+ ATACC[123]
336+
Table "public.atacc1"
337+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
338+
--------+---------+-----------+----------+---------+---------+--------------+-------------
339+
a | integer | | not null | | plain | |
340+
Not-null constraints:
341+
"ditto" NOT NULL "a"
342+
Child tables: atacc2
343+
344+
Table "public.atacc2"
345+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
346+
--------+---------+-----------+----------+---------+---------+--------------+-------------
347+
a | integer | | not null | | plain | |
348+
Not-null constraints:
349+
"a_is_not_null" NOT NULL "a" (local, inherited)
350+
Inherits: atacc1
351+
Child tables: atacc3
352+
353+
Table "public.atacc3"
354+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
355+
--------+---------+-----------+----------+---------+---------+--------------+-------------
356+
a | integer | | not null | | plain | |
357+
Not-null constraints:
358+
"ditto" NOT NULL "a" (inherited)
359+
Inherits: atacc2
360+
361+
ALTER TABLE ATACC2 DROP CONSTRAINT a_is_not_null;
362+
ALTER TABLE ATACC1 DROP CONSTRAINT ditto;
363+
\d+ ATACC3
364+
Table "public.atacc3"
365+
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
366+
--------+---------+-----------+----------+---------+---------+--------------+-------------
367+
a | integer | | | | plain | |
368+
Inherits: atacc2
369+
370+
DROP TABLE ATACC1, ATACC2, ATACC3;
371+
-- The same cannot be achieved this way
372+
CREATE TABLE ATACC2 (a int, CONSTRAINT a_is_not_null NOT NULL a NO INHERIT);
373+
CREATE TABLE ATACC1 (a int, CONSTRAINT ditto NOT NULL a);
374+
CREATE TABLE ATACC3 (a int) INHERITS (ATACC2);
375+
NOTICE: merging column "a" with inherited definition
376+
ALTER TABLE ATACC2 INHERIT ATACC1;
377+
ERROR: cannot add NOT NULL constraint to column "a" of relation "atacc2" with inheritance children
378+
DETAIL: Existing constraint "a_is_not_null" is marked NO INHERIT.
379+
DROP TABLE ATACC1, ATACC2, ATACC3;
324380
--
325381
-- Check constraints on INSERT INTO
326382
--

‎src/test/regress/sql/constraints.sql

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,28 @@ ALTER TABLE ATACC1 ADD NOT NULL a NO INHERIT;
212212
\d+ ATACC2
213213
DROPTABLE ATACC1, ATACC2;
214214

215+
-- overridding a no-inherit constraint with an inheritable one
216+
CREATETABLEATACC2 (aint,CONSTRAINT a_is_not_nullNOT NULL a NO INHERIT);
217+
CREATETABLEATACC1 (aint);
218+
CREATETABLEATACC3 (aint) INHERITS (ATACC2);
219+
INSERT INTO ATACC3VALUES (null);-- make sure we scan atacc3
220+
ALTERTABLE ATACC2 INHERIT ATACC1;
221+
ALTERTABLE ATACC1 ADDCONSTRAINT dittoNOT NULL a;
222+
DELETEFROM ATACC3;
223+
ALTERTABLE ATACC1 ADDCONSTRAINT dittoNOT NULL a;
224+
\d+ ATACC[123]
225+
ALTERTABLE ATACC2 DROPCONSTRAINT a_is_not_null;
226+
ALTERTABLE ATACC1 DROPCONSTRAINT ditto;
227+
\d+ ATACC3
228+
DROPTABLE ATACC1, ATACC2, ATACC3;
229+
230+
-- The same cannot be achieved this way
231+
CREATETABLEATACC2 (aint,CONSTRAINT a_is_not_nullNOT NULL a NO INHERIT);
232+
CREATETABLEATACC1 (aint,CONSTRAINT dittoNOT NULL a);
233+
CREATETABLEATACC3 (aint) INHERITS (ATACC2);
234+
ALTERTABLE ATACC2 INHERIT ATACC1;
235+
DROPTABLE ATACC1, ATACC2, ATACC3;
236+
215237
--
216238
-- Check constraints on INSERT INTO
217239
--

0 commit comments

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