8000 Recast "ONLY" column CHECK constraints as NO INHERIT · jandas/postgres@09ff76f · GitHub
[go: up one dir, main page]
Skip to content

Commit 09ff76f

Browse files
alvherrealvherre
committed
Recast "ONLY" column CHECK constraints as NO INHERIT
The original syntax wasn't universally loved, and it didn't allow its usage in CREATE TABLE, only ALTER TABLE. It now works everywhere, and it also allows using ALTER TABLE ONLY to add an uninherited CHECK constraint, per discussion. The pg_constraint column has accordingly been renamed connoinherit. This commit partly reverts some of the changes in 61d81bd, particularly some pg_dump and psql bits, because now pg_get_constraintdef includes the necessary NO INHERIT within the constraint definition. Author: Nikhil Sontakke Some tweaks by me
1 parent 1f03630 commit 09ff76f

27 files changed

+197
-133
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,8 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
907907
and <literal>TABLESPACE</> actions never recurse to descendant tables;
908908
that is, they always act as though <literal>ONLY</> were specified.
909909
Adding a constraint can recurse only for <literal>CHECK</> constraints,
910-
and is required to do so for such constraints.
910+
and is required to do so for such constraints, except those that are
911+
explicitely marked <literal>NO INHERIT</>.
911912
</para>
912913

913914
<para>
@@ -1013,7 +1014,7 @@ ALTER TABLE distributors ADD CONSTRAINT zipchk CHECK (char_length(zipcode) = 5);
10131014
<para>
10141015
To add a check constraint only to a table and not to its children:
10151016
<programlisting>
1016-
ALTER TABLE ONLY distributors ADD CONSTRAINT zipchk CHECK (char_length(zipcode) = 5);
1017+
ALTER TABLE distributors A F438 DD CONSTRAINT zipchk CHECK NO INHERIT (char_length(zipcode) = 5);
10171018
</programlisting>
10181019
(The check constraint will not be inherited by future children, either.)
10191020
</para>

‎doc/src/sgml/ref/create_table.sgml

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
4747
[ CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable> ]
4848
{ NOT NULL |
4949
NULL |
50-
CHECK ( <replaceable class="PARAMETER">expression</replaceable> ) |
50+
CHECK [ NO INHERIT ] ( <replaceable class="PARAMETER">expression</replaceable> ) |
5151
DEFAULT <replaceable>default_expr</replaceable> |
5252
UNIQUE <replaceable class="PARAMETER">index_parameters</replaceable> |
5353
PRIMARY KEY <replaceable class="PARAMETER">index_parameters</replaceable> |
@@ -58,7 +58,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
5858
<phrase>and <replaceable class="PARAMETER">table_constraint</replaceable> is:</phrase>
5959

6060
[ CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable> ]
61-
{ CHECK ( <replaceable class="PARAMETER">expression</replaceable> ) |
61+
{ CHECK [ NO INHERIT ] ( <replaceable class="PARAMETER">expression</replaceable> ) |
6262
UNIQUE ( <replaceable class="PARAMETER">column_name</replaceable> [, ... ] ) <replaceable class="PARAMETER">index_parameters</replaceable> |
6363
PRIMARY KEY ( <replaceable class="PARAMETER">column_name</replaceable> [, ... ] ) <replaceable class="PARAMETER">index_parameters</replaceable> |
6464
EXCLUDE [ USING <replaceable class="parameter">index_method</replaceable> ] ( <replaceable class="parameter">exclude_element</replaceable> WITH <replaceable class="parameter">operator</replaceable> [, ... ] ) <replaceable class="parameter">index_parameters</replaceable> [ WHERE ( <replaceable class="parameter">predicate</replaceable> ) ] |
@@ -299,7 +299,8 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
299299
contain identically-named <literal>CHECK</> constraints, these
300300
constraints must all have the same check expression, or an error will be
301301
reported. Constraints having the same name and expression will
302-
be merged into one copy. Notice that an unnamed <literal>CHECK</>
302+
be merged into one copy. A constraint marked <literal>NO INHERIT</> in a
303+
parent will not be considered. Notice that an unnamed <literal>CHECK</>
303304
constraint in the new table will never be merged, since a unique name
304305
will always be chosen for it.
305306
</para>
@@ -415,7 +416,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
415416
</varlistentry>
416417

417418
<varlistentry>
418-
<term><literal>CHECK ( <replaceable class="PARAMETER">expression</replaceable> )</literal></term>
419+
<term><literal>CHECK [ NO INHERIT ] ( <replaceable class="PARAMETER">expression</replaceable> )</literal></term>
419420
<listitem>
420421
<para>
421422
The <literal>CHECK</> clause specifies an expression producing a
@@ -434,6 +435,11 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
434435
subqueries nor refer to variables other than columns of the
435436
current row.
436437
</para>
438+
439+
<para>
440+
A constraint marked with <literal>NO INHERIT</> will not propagate to
441+
children tables.
442+
</para>
437443
</listitem>
438444
</varlistentry>
439445

‎src/backend/catalog/heap.c

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,12 @@ static Oid AddNewRelationType(const char *typeName,
9292
Oid new_array_type);
9393
static void RelationRemoveInheritance(Oid relid);
9494
static void StoreRelCheck(Relation rel, char *ccname, Node *expr,
95-
bool is_validated, bool is_local, int inhcount, bool is_only);
95+
bool is_validated, bool is_local, int inhcount,
96+
bool is_no_inherit);
9697
static void StoreConstraints(Relation rel, List *cooked_constraints);
9798
static bool MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
98-
bool allow_merge, bool is_local, bool is_only);
99+
bool allow_merge, bool is_local,
100+
bool is_no_inherit);
99101
static void SetRelationNumChecks(Relation rel, int numchecks);
100102
static Node *cookConstraint(ParseState *pstate,
101103
Node *raw_constraint,
@@ -1868,7 +1870,8 @@ StoreAttrDefault(Relation rel, AttrNumber attnum, Node *expr)
18681870
*/
18691871
static void
18701872
StoreRelCheck(Relation rel, char *ccname, Node *expr,
1871-
bool is_validated, bool is_local, int inhcount, bool is_only)
1873+
bool is_validated, bool is_local, int inhcount,
1874+
bool is_no_inherit)
18721875
{
18731876
char *ccbin;
18741877
char *ccsrc;
@@ -1952,7 +1955,7 @@ StoreRelCheck(Relation rel, char *ccname, Node *expr,
19521955
ccsrc, /* Source form of check constraint */
19531956
is_local, /* conislocal */
19541957
inhcount, /* coninhcount */
1955-
is_only); /* conisonly */
1958+
is_no_inherit); /* connoinherit */
19561959

19571960
pfree(ccbin);
19581961
pfree(ccsrc);
@@ -1993,7 +1996,7 @@ StoreConstraints(Relation rel, List *cooked_constraints)
19931996
break;
19941997
case CONSTR_CHECK:
19951998
StoreRelCheck(rel, con->name, con->expr, !con->skip_validation,
1996-
con->is_local, con->inhcount, con->is_only);
1999+
con->is_local, con->inhcount, con->is_no_inherit);
19972000
numchecks++;
19982001
break;
19992002
default:
@@ -2036,8 +2039,7 @@ AddRelationNewConstraints(Relation rel,
20362039
List *newColDefaults,
20372040
List *newConstraints,
20382041
bool allow_merge,
2039-
bool is_local,
2040-
bool is_only)
2042+
bool is_local)
20412043
{
20422044
List *cookedConstraints = NIL;
20432045
TupleDesc tupleDesc;
@@ -2110,7 +2112,7 @@ AddRelationNewConstraints(Relation rel,
21102112
cooked->skip_validation = false;
21112113
cooked->is_local = is_local;
21122114
cooked->inhcount = is_local ? 0 : 1;
2113-
cooked->is_only = is_only;
2115+
cooked->is_no_inherit = false;
21142116
cookedConstraints = lappend(cookedConstraints, cooked);
21152117
}
21162118

@@ -2178,7 +2180,8 @@ AddRelationNewConstraints(Relation rel,
21782180
* what ATAddCheckConstraint wants.)
21792181
*/
21802182
if (MergeWithExistingConstraint(rel, ccname, expr,
2181-
allow_merge, is_local, is_only))
2183+
allow_merge, is_local,
2184+
cdef->is_no_inherit))
21822185
continue;
21832186
}
21842187
else
@@ -2225,7 +2228,7 @@ AddRelationNewConstraints(Relation rel,
22252228
* OK, store it.
22262229
*/
22272230
StoreRelCheck(rel, ccname, expr, !cdef->skip_validation, is_local,
2228-
is_local ? 0 : 1, is_only);
2231+
is_local ? 0 : 1, cdef->is_no_inherit);
22292232

22302233
numchecks++;
22312234

@@ -2237,7 +2240,7 @@ AddRelationNewConstraints(Relation rel,
22372240
cooked->skip_validation = cdef->skip_validation;
22382241
cooked->is_local = is_local;
22392242
cooked->inhcount = is_local ? 0 : 1;
2240-
cooked->is_only = is_only;
2243+
cooked->is_no_inherit = cdef->is_no_inherit;
22412244
cookedConstraints = lappend(cookedConstraints, cooked);
22422245
}
22432246

@@ -2266,7 +2269,7 @@ AddRelationNewConstraints(Relation rel,
22662269
static bool
22672270
MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
22682271
bool allow_merge, bool is_local,
2269-
bool is_only)
2272+
bool is_no_inherit)
22702273
{
22712274
bool found;
22722275
Relation conDesc;
@@ -2322,8 +2325,8 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
23222325
tup = heap_copytuple(tup);
23232326
con = (Form_pg_constraint) GETSTRUCT(tup);
23242327

2325-
/* If the constraint is "only" then cannot merge */
2326-
if (con->conisonly)
2328+
/* If the constraint is "no inherit" then cannot merge */
2329+
if (con->connoinherit)
23272330
ereport(ERROR,
23282331
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
23292332
errmsg("constraint \"%s\" conflicts with non-inherited constraint on relation \"%s\"",
@@ -2333,10 +2336,10 @@ MergeWithExistingConstraint(Relation rel, char *ccname, Node *expr,
23332336
con->conislocal = true;
23342337
else
23352338
con->coninhcount++;
2336-
if (is_only)
2339+
if (is_no_inherit)
23372340
{
23382341
Assert(is_local);
2339-
con->conisonly = true;
2342+
con->connoinherit = true;
23402343
}
23412344
/* OK to update the tuple */
23422345
ereport(NOTICE,

‎src/backend/catalog/index.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1156,7 +1156,7 @@ index_constraint_create(Relation heapRelation,
11561156
NULL,
11571157
true, /* islocal */
11581158
0, /* inhcount */
1159-
false); /* isonly */
1159+
false); /* noinherit */
11601160

11611161
/*
11621162
* Register the index as internally dependent on the constraint.

‎src/backend/catalog/pg_constraint.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ CreateConstraintEntry(const char *constraintName,
6767
const char *conSrc,
6868
bool conIsLocal,
6969
int conInhCount,
70-
bool conIsOnly)
70+
bool conNoInherit)
7171
{
7272
Relation conDesc;
7373
Oid conOid;
@@ -170,7 +170,7 @@ CreateConstraintEntry(const char *constraintName,
170170
values[Anum_pg_constraint_confmatchtype - 1] = CharGetDatum(foreignMatchType);
171171
values[Anum_pg_constraint_conislocal - 1] = BoolGetDatum(conIsLocal);
172172
values[Anum_pg_constraint_coninhcount - 1] = Int32GetDatum(conInhCount);
173-
values[Anum_pg_constraint_conisonly - 1] = BoolGetDatum(conIsOnly);
173+
values[Anum_pg_constraint_connoinherit - 1] = BoolGetDatum(conNoInherit);
174174

175175
if (conkeyArray)
176176
values[Anum_pg_constraint_conkey - 1] = PointerGetDatum(conkeyArray);

‎src/backend/commands/tablecmds.c

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
601601
cooked->skip_validation = false;
602602
cooked->is_local = true; /* not used for defaults */
603603
cooked->inhcount = 0; /* ditto */
604-
cooked->is_only = false;
604+
cooked->is_no_inherit = false;
605605
cookedDefaults = lappend(cookedDefaults, cooked);
606606
descriptor->attrs[attnum - 1]->atthasdef = true;
607607
}
@@ -661,7 +661,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
661661
*/
662662
if (rawDefaults || stmt->constraints)
663663
AddRelationNewConstraints(rel, rawDefaults, stmt->constraints,
664-
true, true, false);
664+
true, true);
665665

666666
/*
667667
* Clean up. We keep lock on new relation (although it shouldn't be
@@ -1655,7 +1655,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
16551655
Node *expr;
16561656

16571657
/* ignore if the constraint is non-inheritable */
1658-
if (check[i].cconly)
1658+
if (check[i].ccnoinherit)
16591659
continue;
16601660

16611661
/* adjust varattnos of ccbin here */
@@ -1676,7 +1676,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
16761676
cooked->skip_validation = false;
16771677
cooked->is_local = false;
16781678
cooked->inhcount = 1;
1679-
cooked->is_only = false;
1679+
cooked->is_no_inherit = false;
16801680
constraints = lappend(constraints, cooked);
16811681
}
16821682
}
@@ -2399,7 +2399,7 @@ rename_constraint_internal(Oid myrelid,
23992399
constraintOid);
24002400
con = (Form_pg_constraint) GETSTRUCT(tuple);
24012401

2402-
if (myrelid && con->contype == CONSTRAINT_CHECK && !con->conisonly)
2402+
if (myrelid && con->contype == CONSTRAINT_CHECK && !con->connoinherit)
24032403
{
24042404
if (recurse)
24052405
{
@@ -4573,7 +4573,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
45734573
* This function is intended for CREATE TABLE, so it processes a
45744574
* _list_ of defaults, but we just do one.
45754575
*/
4576-
AddRelationNewConstraints(rel, list_make1(rawEnt), NIL, false, true, false);
4576+
AddRelationNewConstraints(rel, list_make1(rawEnt), NIL, false, true);
45774577

45784578
/* Make the additional catalog changes visible */
45794579
CommandCounterIncrement();
@@ -5015,7 +5015,7 @@ ATExecColumnDefault(Relation rel, const char *colName,
50155015
* This function is intended for CREATE TABLE, so it processes a
50165016
* _list_ of defaults, but we just do one.
50175017
*/
5018-
AddRelationNewConstraints(rel, list_make1(rawEnt), NIL, false, true, false);
5018+
AddRelationNewConstraints(rel, list_make1(rawEnt), NIL, false, true);
50195019
}
50205020
}
50215021

@@ -5680,16 +5680,11 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
56805680
* omitted from the returned list, which is what we want: we do not need
56815681
* to do any validation work. That can only happen at child tables,
56825682
* though, since we disallow merging at the top level.
5683-
*
5684-
* Note: we set is_only based on the recurse flag which is false when
5685-
* interpretInhOption() of our statement returns false all the way up
5686-
* in AlterTable and gets passed all the way down to here.
56875683
*/
56885684
newcons = AddRelationNewConstraints(rel, NIL,
56895685
list_make1(copyObject(constr)),
5690-
recursing, /* allow_merge */
5691-
!recursing, /* is_local */
5692-
!recurse && !recursing); /* is_only */
5686+
recursing, /* allow_merge */
5687+
!recursing); /* is_local */
56935688

56945689
/* Add each to-be-validated constraint to Phase 3's queue */
56955690
foreach(lcon, newcons)
@@ -5730,9 +5725,9 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
57305725
return;
57315726

57325727
/*
5733-
* Adding an ONLY constraint? No need to find our children
5728+
* Adding a NO INHERIT constraint? No need to find our children
57345729
*/
5735-
if (!recurse && !recursing)
5730+
if (constr->is_no_inherit)
57365731
return;
57375732

57385733
/*
@@ -5742,6 +5737,16 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
57425737
*/
57435738
children = find_inheritance_children(RelationGetRelid(rel), lockmode);
57445739

5740+
/*
5741+
* Check if ONLY was specified with ALTER TABLE. If so, allow the
5742+
* contraint creation only if there are no children currently. Error out
5743+
* otherwise.
5744+
*/
5745+
if (!recurse && children != NIL)
5746+
ereport(ERROR,
5747+
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
5748+
errmsg("constraint must be added to child tables too")));
5749+
57455750
foreach(child, children)
57465751
{
57475752
Oid childrelid = lfirst_oid(child);
@@ -6127,7 +6132,7 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
61276132
NULL,
61286133
true, /* islocal */
61296134
0, /* inhcount */
6130-
false); /* isonly */
6135+
false); /* isnoinherit */
61316136

61326137
/*
61336138
* Create the triggers that will enforce the constraint.
@@ -6998,8 +7003,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
69987003
ScanKeyData key;
69997004
HeapTuple tuple;
70007005
bool found = false;
7001-
bool is_check_constraint = false;
7002-
bool is_only_constraint = false;
7006+
bool is_no_inherit_constraint = false;
70037007

70047008
/* At top level, permission check was done in ATPrepCmd, else do it */
70057009
if (recursing)
@@ -7033,15 +7037,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
70337037
errmsg("cannot drop inherited constraint \"%s\" of relation \"%s\"",
70347038
constrName, RelationGetRelationName(rel))));
70357039

7036-
/* Right now only CHECK constraints can be inherited */
7037-
if (con->contype == CONSTRAINT_CHECK)
7038-
is_check_constraint = true;
7039-
7040-
if (con->conisonly)
7041-
{
7042-
Assert(is_check_constraint);
7043-
is_only_constraint = true;
7044-
}
7040+
is_no_inherit_constraint = con->connoinherit;
70457041

70467042
/*
70477043
* Perform the actual constraint deletion
@@ -7084,7 +7080,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
70847080
* routines, we have to do this one level of recursion at a time; we can't
70857081
* use find_all_inheritors to do it in one pass.
70867082
*/
7087-
if (is_check_constraint && !is_only_constraint)
7083+
if (!is_no_inherit_constraint)
70887084
children = find_inheritance_children(RelationGetRelid(rel), lockmode);
70897085
else
70907086
children = NIL;
@@ -9250,8 +9246,8 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
92509246
if (parent_con->contype != CONSTRAINT_CHECK)
92519247
continue;
92529248

9253-
/* if the parent's constraint is marked ONLY, it's not inherited */
9254-
if (parent_con->conisonly)
9249+
/* if the parent's constraint is marked NO INHERIT, it's not inherited */
9250+
if (parent_con->connoinherit)
92559251
continue;
92569252

92579253
/* Search for a child constraint matching this one */
@@ -9281,8 +9277,8 @@ MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel)
92819277
RelationGetRelationName(child_rel),
92829278
NameStr(parent_con->conname))));
92839279

9284-
/* If the constraint is "only" then cannot merge */
9285-
if (child_con->conisonly)
9280+
/* If the constraint is "no inherit" then cannot merge */
9281+
if (child_con->connoinherit)
92869282
ereport(ERROR,
92879283
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
92889284
errmsg("constraint \"%s\" conflicts with non-inherited constraint on child table \"%s\"",

0 commit comments

Comments
 (0)
0