Skip to content

Commit e5f455f

Browse files
tglsfdctglsfdc
committed
Apply table and domain CHECK constraints in name order.
Previously, CHECK constraints of the same scope were checked in whatever order they happened to be read from pg_constraint. (Usually, but not reliably, this would be creation order for domain constraints and reverse creation order for table constraints, because of differing implementation details.) Nondeterministic results of this sort are problematic at least for testing purposes, and in discussion it was agreed to be a violation of the principle of least astonishment. Therefore, borrow the principle already established for triggers, and apply such checks in name order (using strcmp() sort rules). This lets users control the check order if they have a mind to. Domain CHECK constraints still follow the rule of checking lower nested domains' constraints first; the name sort only applies to multiple constraints attached to the same domain. In passing, I failed to resist the temptation to wordsmith a bit in create_domain.sgml. Apply to HEAD only, since this could result in a behavioral change in existing applications, and the potential regression test failures have not actually been observed in our buildfarm.
1 parent 871293f commit e5f455f

File tree

6 files changed

+128
-23
lines changed

6 files changed

+128
-23
lines changed

‎doc/src/sgml/ref/create_domain.sgml

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,8 @@ CREATE DOMAIN <replaceable class="parameter">name</replaceable> [ AS ] <replacea
139139
<term><literal>NOT NULL</></term>
140140
<listitem>
141141
<para>
142-
Values of this domain are normally prevented from being null.
143-
However, it is still possible for a domain with this constraint
144-
to take a null value if it is assigned a matching domain type
145-
that has become null, e.g. via a LEFT OUTER JOIN, or
146-
<command>INSERT INTO tab (domcol) VALUES ((SELECT domcol FROM
147-
tab WHERE false))</command>.
142+
Values of this domain are prevented from being null
143+
(but see notes below).
148144
</para>
149145
</listitem>
150146
</varlistentry>
@@ -171,18 +167,55 @@ CREATE DOMAIN <replaceable class="parameter">name</replaceable> [ AS ] <replacea
171167
which values of the domain must satisfy.
172168
Each constraint must be an expression
173169
producing a Boolean result. It should use the key word <literal>VALUE</>
174-
to refer to the value being tested.
170+
to refer to the value being tested. Expressions evaluating
171+
to TRUE or UNKNOWN succeed. If the expression produces a FALSE result,
172+
an error is reported and the value is not allowed to be converted
173+
to the domain type.
175174
</para>
176175

177176
<para>
178177
Currently, <literal>CHECK</literal> expressions cannot contain
179178
subqueries nor refer to variables other than <literal>VALUE</>.
180179
</para>
180+
181+
<para>
182+
When a domain has multiple <literal>CHECK</literal> constraints,
183+
they will be tested in alphabetical order by name.
184+
(<productname>PostgreSQL</> versions before 9.5 did not honor any
185+
particular firing order for <literal>CHECK</literal> constraints.)
186+
</para>
181187
</listitem>
182188
</varlistentry>
183189
</variablelist>
184190
</refsect1>
185191

192+
<refsect1>
193+
<title>Notes</title>
194+
195+
<para>
196+
Domain constraints, particularly <literal>NOT NULL</>, are checked when
197+
converting a value to the domain type. It is possible for a column that
198+
is nominally of the domain type to read as null despite there being such
199+
a constraint. For example, this can happen in an outer-join query, if
200+
the domain column is on the nullable side of the outer join. A more
201+
subtle example is
202+
<programlisting>
203+
INSERT INTO tab (domcol) VALUES ((SELECT domcol FROM tab WHERE false));
204+
</programlisting>
205+
The empty scalar sub-SELECT will produce a null value that is considered
206+
to be of the domain type, so no further constraint checking is applied
207+
to it, and the insertion will succeed.
208+
</para>
209+
210+
<para>
211+
It is very difficult to avoid such problems, because of SQL's general
212+
assumption that NULL is a valid value of every datatype. Best practice
213+
therefore is to design a domain's constraints so that NULL is allowed,
214+
and then to apply column <literal>NOT NULL</> constraints to columns of
215+
the domain type as needed, rather than directly to the domain type.
216+
</para>
217+
</refsect1>
218+
186219
<refsect1>
187220
<title>Examples</title>
188221

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,14 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
442442
A constraint marked with <literal>NO INHERIT</> will not propagate to
443443
child tables.
444444
</para>
445+
446+
<para>
447+
When a table has multiple <literal>CHECK</literal> constraints,
448+
they will be tested for each row in alphabetical order by name,
449+
after checking <literal>NOT NULL</> constraints.
450+
(<productname>PostgreSQL</> versions before 9.5 did not honor any
451+
particular firing order for <literal>CHECK</literal> constraints.)
452+
</para>
445453
</listitem>
446454
</varlistentry>
447455

‎src/backend/utils/cache/relcache.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ static TupleDesc GetPgClassDescriptor(void);
271271
static TupleDesc GetPgIndexDescriptor(void);
272272
static void AttrDefaultFetch(Relation relation);
273273
static void CheckConstraintFetch(Relation relation);
274+
static int CheckConstraintCmp(const void *a, const void *b);
274275
static List *insert_ordered_oid(List *list, Oid datum);
275276
static void IndexSupportInitialize(oidvector *indclass,
276277
RegProcedure *indexSupport,
@@ -3734,6 +3735,22 @@ CheckConstraintFetch(Relation relation)
37343735
if (found != ncheck)
37353736
elog(ERROR, "%d constraint record(s) missing for rel %s",
37363737
ncheck - found, RelationGetRelationName(relation));
3738+
3739+
/* Sort the records so that CHECKs are applied in a deterministic order */
3740+
if (ncheck > 1)
3741+
qsort(check, ncheck, sizeof(ConstrCheck), CheckConstraintCmp);
3742+
}
3743+
3744+
/*
3745+
* qsort comparator to sort ConstrCheck entries by name
3746+
*/
3747+
static int
3748+
CheckConstraintCmp(const void *a, const void *b)
3749+
{
3750+
const ConstrCheck *ca = (const ConstrCheck *) a;
3751+
const ConstrCheck *cb = (const ConstrCheck *) b;
3752+
3753+
return strcmp(ca->ccname, cb->ccname);
37373754
}
37383755

37393756
/*

‎src/backend/utils/cache/typcache.c

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ static int32 NextRecordTypmod = 0; /* number of entries used */
149149
static void load_typcache_tupdesc(TypeCacheEntry *typentry);
150150
static void load_rangetype_info(TypeCacheEntry *typentry);
151151
static void load_domaintype_info(TypeCacheEntry *typentry);
152+
static int dcs_cmp(const void *a, const void *b);
152153
static void decr_dcc_refcount(DomainConstraintCache *dcc);
153154
static void dccref_deletion_callback(void *arg);
154155
static bool array_element_has_equality(TypeCacheEntry *typentry);
@@ -650,6 +651,8 @@ load_domaintype_info(TypeCacheEntry *typentry)
650651
Oid typeOid = typentry->type_id;
651652
DomainConstraintCache *dcc;
652653
bool notNull = false;
654+
DomainConstraintState **ccons;
655+
int cconslen;
653656
Relation conRel;
654657
MemoryContext oldcxt;
655658

@@ -666,9 +669,12 @@ load_domaintype_info(TypeCacheEntry *typentry)
666669

667670
/*
668671
* We try to optimize the common case of no domain constraints, so don't
669-
* create the dcc object and context until we find a constraint.
672+
* create the dcc object and context until we find a constraint. Likewise
673+
* for the temp sorting array.
670674
*/
671675
dcc = NULL;
676+
ccons = NULL;
677+
cconslen = 0;
672678

673679
/*
674680
* Scan pg_constraint for relevant constraints. We want to find
@@ -682,6 +688,7 @@ load_domaintype_info(TypeCacheEntry *typentry)
682688
HeapTuple tup;
683689
HeapTuple conTup;
684690
Form_pg_type typTup;
691+
int nccons = 0;
685692
ScanKeyData key[1];
686693
SysScanDesc scan;
687694

@@ -763,17 +770,45 @@ load_domaintype_info(TypeCacheEntry *typentry)
763770
r->name = pstrdup(NameStr(c->conname));
764771
r->check_expr = ExecInitExpr(check_expr, NULL);
765772

773+
MemoryContextSwitchTo(oldcxt);
774+
775+
/* Accumulate constraints in an array, for sorting below */
776+
if (ccons == NULL)
777+
{
778+
cconslen = 8;
779+
ccons = (DomainConstraintState **)
780+
palloc(cconslen * sizeof(DomainConstraintState *));
781+
}
782+
else if (nccons >= cconslen)
783+
{
784+
cconslen *= 2;
785+
ccons = (DomainConstraintState **)
786+
repalloc(ccons, cconslen * sizeof(DomainConstraintState *));
787+
}
788+
ccons[nccons++] = r;
789+
}
790+
791+
systable_endscan(scan);
792+
793+
if (nccons > 0)
794+
{
766795
/*
767-
* Use lcons() here because constraints of parent domains should
768-
* be applied earlier.
796+
* Sort the items for this domain, so that CHECKs are applied in a
797+
* deterministic order.
769798
*/
770-
dcc->constraints = lcons(r, dcc->constraints);
799+
if (nccons > 1)
800+
qsort(ccons, nccons, sizeof(DomainConstraintState *), dcs_cmp);
771801

802+
/*
803+
* Now attach them to the overall list. Use lcons() here because
804+
* constraints of parent domains should be applied earlier.
805+
*/
806+
oldcxt = MemoryContextSwitchTo(dcc->dccContext);
807+
while (nccons > 0)
808+
dcc->constraints = lcons(ccons[--nccons], dcc->constraints);
772809
MemoryContextSwitchTo(oldcxt);
773810
}
774811

775-
systable_endscan(scan);
776-
777812
/* loop to next domain in stack */
778813
typeOid = typTup->typbasetype;
779814
ReleaseSysCache(tup);
@@ -836,6 +871,18 @@ load_domaintype_info(TypeCacheEntry *typentry)
836871
typentry->flags |= TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS;
837872
}
838873

874+
/*
875+
* qsort comparator to sort DomainConstraintState pointers by name
876+
*/
877+
static int
878+
dcs_cmp(const void *a, const void *b)
879+
{
880+
const DomainConstraintState *const * ca = (const DomainConstraintState *const *) a;
881+
const DomainConstraintState *const * cb = (const DomainConstraintState *const *) b;
882+
883+
return strcmp((*ca)->name, (*cb)->name);
884+
}
885+
839886
/*
840887
* decr_dcc_refcount --- decrement a DomainConstraintCache's refcount,
841888
* and free it if no references remain

‎src/test/regress/input/constraints.source

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ CREATE SEQUENCE INSERT_SEQ;
8787
CREATE TABLE INSERT_TBL (x INT DEFAULT nextval('insert_seq'),
8888
y TEXT DEFAULT '-NULL-',
8989
z INT DEFAULT -1 * currval('insert_seq'),
90-
CONSTRAINT INSERT_CON CHECK (x >= 3 AND y <> 'check failed' AND x < 8),
90+
CONSTRAINT INSERT_TBL_CON CHECK (x >= 3 AND y <> 'check failed' AND x < 8),
9191
CHECK (x + z = 0));
9292

9393
INSERT INTO INSERT_TBL(x,z) VALUES (2, -2);

‎src/test/regress/output/constraints.source

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,10 @@ CREATE SEQUENCE INSERT_SEQ;
116116
CREATE TABLE INSERT_TBL (x INT DEFAULT nextval('insert_seq'),
117117
y TEXT DEFAULT '-NULL-',
118118
z INT DEFAULT -1 * currval('insert_seq'),
119-
CONSTRAINT INSERT_CON CHECK (x >= 3 AND y <> 'check failed' AND x < 8),
119+
CONSTRAINT INSERT_TBL_CON CHECK (x >= 3 AND y <> 'check failed' AND x < 8),
120120
CHECK (x + z = 0));
121121
INSERT INTO INSERT_TBL(x,z) VALUES (2, -2);
122-
ERROR: new row for relation "insert_tbl" violates check constraint "insert_con"
122+
ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con"
123123
DETAIL: Failing row contains (2, -NULL-, -2).
124124
SELECT '' AS zero, * FROM INSERT_TBL;
125125
zero | x | y | z
@@ -133,15 +133,15 @@ SELECT 'one' AS one, nextval('insert_seq');
133133
(1 row)
134134

135135
INSERT INTO INSERT_TBL(y) VALUES ('Y');
136-
ERROR: new row for relation "insert_tbl" violates check constraint "insert_con"
136+
ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con"
137137
DETAIL: Failing row contains (2, Y, -2).
138138
INSERT INTO INSERT_TBL(y) VALUES ('Y');
139139
INSERT INTO INSERT_TBL(x,z) VALUES (1, -2);
140140
ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_check"
141141
DETAIL: Failing row contains (1, -NULL-, -2).
142142
INSERT INTO INSERT_TBL(z,x) VALUES (-7, 7);
143143
INSERT INTO INSERT_TBL VALUES (5, 'check failed', -5);
144-
ERROR: new row for relation "insert_tbl" violates check constraint "insert_con"
144+
ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con"
145145
DETAIL: Failing row contains (5, check failed, -5).
146146
INSERT INTO INSERT_TBL VALUES (7, '!check failed', -7);
147147
INSERT INTO INSERT_TBL(y) VALUES ('-!NULL-');
@@ -158,7 +158,7 @@ INSERT INTO INSERT_TBL(y,z) VALUES ('check failed', 4);
158158
ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_check"
159159
DETAIL: Failing row contains (5, check failed, 4).
160160
INSERT INTO INSERT_TBL(x,y) VALUES (5, 'check failed');
161-
ERROR: new row for relation "insert_tbl" violates check constraint "insert_con"
161+
ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con"
162162
DETAIL: Failing row contains (5, check failed, -5).
163163
INSERT INTO INSERT_TBL(x,y) VALUES (5, '!check failed');
164164
INSERT INTO INSERT_TBL(y) VALUES ('-!NULL-');
@@ -180,7 +180,7 @@ SELECT 'seven' AS one, nextval('insert_seq');
180180
(1 row)
181181

182182
INSERT INTO INSERT_TBL(y) VALUES ('Y');
183-
ERROR: new row for relation "insert_tbl" violates check constraint "insert_con"
183+
ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con"
184184
DETAIL: Failing row contains (8, Y, -8).
185185
SELECT 'eight' AS one, currval('insert_seq');
186186
one | currval
@@ -242,7 +242,7 @@ INSERT INTO INSERT_CHILD(x,z,cy) VALUES (6,-7,7);
242242
ERROR: new row for relation "insert_child" violates check constraint "insert_tbl_check"
243243
DETAIL: Failing row contains (6, -NULL-, -7, 42, 7).
244244
INSERT INTO INSERT_CHILD(x,y,z,cy) VALUES (6,'check failed',-6,7);
245-
ERROR: new row for relation "insert_child" violates check constraint "insert_con"
245+
ERROR: new row for relation "insert_child" violates check constraint "insert_tbl_con"
246246
DETAIL: Failing row contains (6, check failed, -6, 42, 7).
247247
SELECT * FROM INSERT_CHILD;
248248
x | y | z | cx | cy
@@ -305,7 +305,7 @@ SELECT '' AS three, * FROM INSERT_TBL;
305305
INSERT INTO INSERT_TBL SELECT * FROM tmp WHERE yd = 'try again';
306306
INSERT INTO INSERT_TBL(y,z) SELECT yd, -7 FROM tmp WHERE yd = 'try again';
307307
INSERT INTO INSERT_TBL(y,z) SELECT yd, -8 FROM tmp WHERE yd = 'try again';
308-
ERROR: new row for relation "insert_tbl" violates check constraint "insert_con"
308+
ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con"
309309
DETAIL: Failing row contains (8, try again, -8).
310310
SELECT '' AS four, * FROM INSERT_TBL;
311311
four | x | y | z
@@ -325,7 +325,7 @@ UPDATE INSERT_TBL SET x = NULL WHERE x = 5;
325325
UPDATE INSERT_TBL SET x = 6 WHERE x = 6;
326326
UPDATE INSERT_TBL SET x = -z, z = -x;
327327
UPDATE INSERT_TBL SET x = z, z = x;
328-
ERROR: new row for relation "insert_tbl" violates check constraint "insert_con"
328+
ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con"
329329
DETAIL: Failing row contains (-4, Y, 4).
330330
SELECT * FROM INSERT_TBL;
331331
x | y | z

0 commit comments

Comments
 (0)