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

Commitd45597f

Browse files
committed
Disallow direct change of NO INHERIT of not-null constraints
We support changing NO INHERIT constraint to INHERIT for constraints inchild relations when adding a constraint to some ancestor relation, andalso during pg_upgrade's schema restore; but other than those specialcases, command ALTER TABLE ADD CONSTRAINT should not be allowed tochange an existing constraint from NO INHERIT to INHERIT, as that wouldrequire to process child relations so that they also acquire anappropriate constraint, which we may not be in a position to do. (It'dalso be surprising behavior.)It is conceivable that we want to allow ALTER TABLE SET NOT NULL to makesuch a change; but in that case some more code is needed to implement itcorrectly, so for now I've made that throw the same error message.Also, during the prep phase of ALTER TABLE ADD CONSTRAINT, acquire lockson all descendant tables; otherwise we might operate on child tables onwhich no locks are held, particularly in the mode where a primary keycauses not-null constraints to be created on children.Reported-by: Alexander Lakhin <exclusion@gmail.com>Discussion:https://postgr.es/m/7d923a66-55f0-3395-cd40-81c142b5448b@gmail.com
1 parent42510c0 commitd45597f

File tree

7 files changed

+92
-14
lines changed

7 files changed

+92
-14
lines changed

‎doc/src/sgml/ref/alter_table.sgml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
105105
<phrase>and <replaceable class="parameter">column_constraint</replaceable> is:</phrase>
106106

107107
[ CONSTRAINT <replaceable class="parameter">constraint_name</replaceable> ]
108-
{ NOT NULL |
108+
{ NOT NULL[ NO INHERIT ]|
109109
NULL |
110110
CHECK ( <replaceable class="parameter">expression</replaceable> ) [ NO INHERIT ] |
111111
DEFAULT <replaceable>default_expr</replaceable> |

‎src/backend/catalog/heap.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2549,13 +2549,23 @@ AddRelationNewConstraints(Relation rel,
25492549

25502550
/*
25512551
* 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.
2552+
* we need only adjust its coninhcount and we're done. In certain
2553+
* cases (see below), if the constraint is there but marked NO
2554+
* INHERIT, then we mark it as no longer such and coninhcount
2555+
* updated, plus we must also recurse to the children (if any) to
2556+
* add the constraint there.
2557+
*
2558+
* We only allow the inheritability status to change during binary
2559+
* upgrade (where it's used to add the not-null constraints for
2560+
* children of tables with primary keys), or when we're recursing
2561+
* processing a table down an inheritance hierarchy; directly
2562+
* allowing a constraint to change from NO INHERIT to INHERIT
2563+
* during ALTER TABLE ADD CONSTRAINT would be far too surprising
2564+
* behavior.
25562565
*/
25572566
existing=AdjustNotNullInheritance1(RelationGetRelid(rel),colnum,
2558-
cdef->inhcount,cdef->is_no_inherit);
2567+
cdef->inhcount,cdef->is_no_inherit,
2568+
IsBinaryUpgrade||allow_merge);
25592569
if (existing==1)
25602570
continue;/* all done! */
25612571
elseif (existing==-1)

‎src/backend/catalog/pg_constraint.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ extractNotNullColumn(HeapTuple constrTup)
721721
*/
722722
int
723723
AdjustNotNullInheritance1(Oidrelid,AttrNumberattnum,intcount,
724-
boolis_no_inherit)
724+
boolis_no_inherit,boolallow_noinherit_change)
725725
{
726726
HeapTupletup;
727727

@@ -744,16 +744,23 @@ AdjustNotNullInheritance1(Oid relid, AttrNumber attnum, int count,
744744
if (is_no_inherit&& !conform->connoinherit)
745745
ereport(ERROR,
746746
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
747-
errmsg("cannot change NO INHERIT status ofinheritedNOT NULL constraint \"%s\" on relation \"%s\"",
747+
errmsg("cannot change NO INHERIT status of NOT NULL constraint \"%s\" on relation \"%s\"",
748748
NameStr(conform->conname),get_rel_name(relid)));
749749

750750
/*
751751
* If the constraint already exists in this relation but it's marked
752-
* NO INHERIT, we can just remove that flag, and instruct caller to
753-
* recurse to add the constraint to children.
752+
* NO INHERIT, we can just remove that flag (provided caller allows
753+
* such a change), and instruct caller to recurse to add the
754+
* constraint to children.
754755
*/
755756
if (!is_no_inherit&&conform->connoinherit)
756757
{
758+
if (!allow_noinherit_change)
759+
ereport(ERROR,
760+
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
761+
errmsg("cannot change NO INHERIT status of NOT NULL constraint \"%s\" in relation \"%s\"",
762+
NameStr(conform->conname),get_rel_name(relid)));
763+
757764
conform->connoinherit= false;
758765
retval=-1;/* caller must add constraint on child rels */
759766
}

‎src/backend/commands/tablecmds.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4980,10 +4980,12 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
49804980
break;
49814981
case AT_AddConstraint:/* ADD CONSTRAINT */
49824982
ATSimplePermissions(cmd->subtype, rel, ATT_TABLE | ATT_FOREIGN_TABLE);
4983-
/* Recursion occurs during execution phase */
4984-
/* No command-specific prep needed except saving recurse flag */
49854983
if (recurse)
4984+
{
4985+
/* recurses at exec time; lock descendants and set flag */
4986+
(void) find_all_inheritors(RelationGetRelid(rel), lockmode, NULL);
49864987
cmd->recurse = true;
4988+
}
49874989
pass = AT_PASS_ADD_CONSTR;
49884990
break;
49894991
case AT_AddIndexConstraint: /* ADD CONSTRAINT USING INDEX */
@@ -7892,6 +7894,17 @@ ATExecSetNotNull(List **wqueue, Relation rel, char *conName, char *colName,
78927894
copytup = heap_copytuple(tuple);
78937895
conForm = (Form_pg_constraint) GETSTRUCT(copytup);
78947896

7897+
/*
7898+
* Don't let a NO INHERIT constraint be changed into inherit.
7899+
*/
7900+
if (conForm->connoinherit && recurse)
7901+
ereport(ERROR,
7902+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
7903+
errmsg("cannot change NO INHERIT status of NOT NULL constraint \"%s\" in relation \"%s\"",
7904+
NameStr(conForm->conname),
7905+
RelationGetRelationName(rel)));
7906+
7907+
78957908
/*
78967909
* If we find an appropriate constraint, we're almost done, but just
78977910
* need to change some properties on it: if we're recursing, increment

‎src/include/catalog/pg_constraint.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ extern HeapTuple findNotNullConstraint(Oid relid, const char *colname);
262262
externHeapTuplefindDomainNotNullConstraint(Oidtypid);
263263
externAttrNumberextractNotNullColumn(HeapTupleconstrTup);
264264
externintAdjustNotNullInheritance1(Oidrelid,AttrNumberattnum,intcount,
265-
boolis_no_inherit);
265+
boolis_no_inherit,boolallow_noinherit_change);
266266
externvoidAdjustNotNullInheritance(Oidrelid,Bitmapset*columns,intcount);
267267
externList*RelationGetNotNullConstraints(Oidrelid,boolcooked);
268268

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2126,7 +2126,7 @@ ERROR: cannot define not-null constraint on column "a2" with NO INHERIT
21262126
DETAIL: The column has an inherited not-null constraint.
21272127
-- change NO INHERIT status of inherited constraint: no dice, it's inherited
21282128
alter table cc2 add not null a2 no inherit;
2129-
ERROR: cannot change NO INHERIT status ofinheritedNOT NULL constraint "nn" on relation "cc2"
2129+
ERROR: cannot change NO INHERIT status of NOT NULL constraint "nn" on relation "cc2"
21302130
-- remove constraint from cc2: no dice, it's inherited
21312131
alter table cc2 alter column a2 drop not null;
21322132
ERROR: cannot drop inherited constraint "nn" of relation "cc2"
@@ -2277,6 +2277,34 @@ Child tables: inh_nn_child,
22772277
inh_nn_child2
22782278

22792279
drop table inh_nn_parent, inh_nn_child, inh_nn_child2;
2280+
CREATE TABLE inh_nn_parent (a int, NOT NULL a NO INHERIT);
2281+
CREATE TABLE inh_nn_child() INHERITS (inh_nn_parent);
2282+
ALTER TABLE inh_nn_parent ADD CONSTRAINT nna NOT NULL a;
2283+
ERROR: cannot change NO INHERIT status of NOT NULL constraint "inh_nn_parent_a_not_null" in relation "inh_nn_parent"
2284+
ALTER TABLE inh_nn_parent ALTER a SET NOT NULL;
2285+
ERROR: cannot change NO INHERIT status of NOT NULL constraint "inh_nn_parent_a_not_null" in relation "inh_nn_parent"
2286+
DROP TABLE inh_nn_parent cascade;
2287+
NOTICE: drop cascades to table inh_nn_child
2288+
-- Adding a PK at the top level of a hierarchy should cause all descendants
2289+
-- to be checked for nulls, even past a no-inherit constraint
2290+
CREATE TABLE inh_nn_lvl1 (a int);
2291+
CREATE TABLE inh_nn_lvl2 () INHERITS (inh_nn_lvl1);
2292+
CREATE TABLE inh_nn_lvl3 (CONSTRAINT foo NOT NULL a NO INHERIT) INHERITS (inh_nn_lvl2);
2293+
CREATE TABLE inh_nn_lvl4 () INHERITS (inh_nn_lvl3);
2294+
CREATE TABLE inh_nn_lvl5 () INHERITS (inh_nn_lvl4);
2295+
INSERT INTO inh_nn_lvl2 VALUES (NULL);
2296+
ALTER TABLE inh_nn_lvl1 ADD PRIMARY KEY (a);
2297+
ERROR: column "a" of relation "inh_nn_lvl2" contains null values
2298+
DELETE FROM inh_nn_lvl2;
2299+
INSERT INTO inh_nn_lvl5 VALUES (NULL);
2300+
ALTER TABLE inh_nn_lvl1 ADD PRIMARY KEY (a);
2301+
ERROR: column "a" of relation "inh_nn_lvl5" contains null values
2302+
DROP TABLE inh_nn_lvl1 CASCADE;
2303+
NOTICE: drop cascades to 4 other objects
2304+
DETAIL: drop cascades to table inh_nn_lvl2
2305+
drop cascades to table inh_nn_lvl3
2306+
drop cascades to table inh_nn_lvl4
2307+
drop cascades to table inh_nn_lvl5
22802308
--
22812309
-- test inherit/deinherit
22822310
--

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,26 @@ select conrelid::regclass, conname, contype, conkey,
844844
\d+ inh_nn*
845845
droptable inh_nn_parent, inh_nn_child, inh_nn_child2;
846846

847+
CREATETABLEinh_nn_parent (aint,NOT NULL a NO INHERIT);
848+
CREATETABLEinh_nn_child() INHERITS (inh_nn_parent);
849+
ALTERTABLE inh_nn_parent ADDCONSTRAINT nnaNOT NULL a;
850+
ALTERTABLE inh_nn_parent ALTER aSETNOT NULL;
851+
DROPTABLE inh_nn_parent cascade;
852+
853+
-- Adding a PK at the top level of a hierarchy should cause all descendants
854+
-- to be checked for nulls, even past a no-inherit constraint
855+
CREATETABLEinh_nn_lvl1 (aint);
856+
CREATETABLEinh_nn_lvl2 () INHERITS (inh_nn_lvl1);
857+
CREATETABLEinh_nn_lvl3 (CONSTRAINT fooNOT NULL a NO INHERIT) INHERITS (inh_nn_lvl2);
858+
CREATETABLEinh_nn_lvl4 () INHERITS (inh_nn_lvl3);
859+
CREATETABLEinh_nn_lvl5 () INHERITS (inh_nn_lvl4);
860+
INSERT INTO inh_nn_lvl2VALUES (NULL);
861+
ALTERTABLE inh_nn_lvl1 ADDPRIMARY KEY (a);
862+
DELETEFROM inh_nn_lvl2;
863+
INSERT INTO inh_nn_lvl5VALUES (NULL);
864+
ALTERTABLE inh_nn_lvl1 ADDPRIMARY KEY (a);
865+
DROPTABLE inh_nn_lvl1 CASCADE;
866+
847867
--
848868
-- test inherit/deinherit
849869
--

0 commit comments

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