From e5f455f59fed0632371cddacddd79895b148dc07 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 23 Mar 2015 16:59:29 -0400 Subject: [PATCH] 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. --- doc/src/sgml/ref/create_domain.sgml | 47 ++++++++++++++--- doc/src/sgml/ref/create_table.sgml | 8 +++ src/backend/utils/cache/relcache.c | 17 ++++++ src/backend/utils/cache/typcache.c | 61 +++++++++++++++++++--- src/test/regress/input/constraints.source | 2 +- src/test/regress/output/constraints.source | 18 +++---- 6 files changed, 129 insertions(+), 24 deletions(-) diff --git a/doc/src/sgml/ref/create_domain.sgml b/doc/src/sgml/ref/create_domain.sgml index dd782e48dc..8b7848b820 100644 --- a/doc/src/sgml/ref/create_domain.sgml +++ b/doc/src/sgml/ref/create_domain.sgml @@ -139,12 +139,8 @@ CREATE DOMAIN name [ AS ] NOT NULL - Values of this domain are normally prevented from being null. - However, it is still possible for a domain with this constraint - to take a null value if it is assigned a matching domain type - that has become null, e.g. via a LEFT OUTER JOIN, or - INSERT INTO tab (domcol) VALUES ((SELECT domcol FROM - tab WHERE false)). + Values of this domain are prevented from being null + (but see notes below). @@ -171,18 +167,55 @@ CREATE DOMAIN name [ AS ] VALUE - to refer to the value being tested. + to refer to the value being tested. Expressions evaluating + to TRUE or UNKNOWN succeed. If the expression produces a FALSE result, + an error is reported and the value is not allowed to be converted + to the domain type. Currently, CHECK expressions cannot contain subqueries nor refer to variables other than VALUE. + + + When a domain has multiple CHECK constraints, + they will be tested in alphabetical order by name. + (PostgreSQL versions before 9.5 did not honor any + particular firing order for CHECK constraints.) + + + Notes + + + Domain constraints, particularly NOT NULL, are checked when + converting a value to the domain type. It is possible for a column that + is nominally of the domain type to read as null despite there being such + a constraint. For example, this can happen in an outer-join query, if + the domain column is on the nullable side of the outer join. A more + subtle example is + +INSERT INTO tab (domcol) VALUES ((SELECT domcol FROM tab WHERE false)); + + The empty scalar sub-SELECT will produce a null value that is considered + to be of the domain type, so no further constraint checking is applied + to it, and the insertion will succeed. + + + + It is very difficult to avoid such problems, because of SQL's general + assumption that NULL is a valid value of every datatype. Best practice + therefore is to design a domain's constraints so that NULL is allowed, + and then to apply column NOT NULL constraints to columns of + the domain type as needed, rather than directly to the domain type. + + + Examples diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 3e6246da66..324d59371a 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -442,6 +442,14 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI A constraint marked with NO INHERIT will not propagate to child tables. + + + When a table has multiple CHECK constraints, + they will be tested for each row in alphabetical order by name, + after checking NOT NULL constraints. + (PostgreSQL versions before 9.5 did not honor any + particular firing order for CHECK constraints.) + diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 1db4ba8410..4ea01d1ad6 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -271,6 +271,7 @@ static TupleDesc GetPgClassDescriptor(void); static TupleDesc GetPgIndexDescriptor(void); static void AttrDefaultFetch(Relation relation); static void CheckConstraintFetch(Relation relation); +static int CheckConstraintCmp(const void *a, const void *b); static List *insert_ordered_oid(List *list, Oid datum); static void IndexSupportInitialize(oidvector *indclass, RegProcedure *indexSupport, @@ -3734,6 +3735,22 @@ CheckConstraintFetch(Relation relation) if (found != ncheck) elog(ERROR, "%d constraint record(s) missing for rel %s", ncheck - found, RelationGetRelationName(relation)); + + /* Sort the records so that CHECKs are applied in a deterministic order */ + if (ncheck > 1) + qsort(check, ncheck, sizeof(ConstrCheck), CheckConstraintCmp); +} + +/* + * qsort comparator to sort ConstrCheck entries by name + */ +static int +CheckConstraintCmp(const void *a, const void *b) +{ + const ConstrCheck *ca = (const ConstrCheck *) a; + const ConstrCheck *cb = (const ConstrCheck *) b; + + return strcmp(ca->ccname, cb->ccname); } /* diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c index 44b5937415..57257d281d 100644 --- a/src/backend/utils/cache/typcache.c +++ b/src/backend/utils/cache/typcache.c @@ -149,6 +149,7 @@ static int32 NextRecordTypmod = 0; /* number of entries used */ static void load_typcache_tupdesc(TypeCacheEntry *typentry); static void load_rangetype_info(TypeCacheEntry *typentry); static void load_domaintype_info(TypeCacheEntry *typentry); +static int dcs_cmp(const void *a, const void *b); static void decr_dcc_refcount(DomainConstraintCache *dcc); static void dccref_deletion_callback(void *arg); static bool array_element_has_equality(TypeCacheEntry *typentry); @@ -650,6 +651,8 @@ load_domaintype_info(TypeCacheEntry *typentry) Oid typeOid = typentry->type_id; DomainConstraintCache *dcc; bool notNull = false; + DomainConstraintState **ccons; + int cconslen; Relation conRel; MemoryContext oldcxt; @@ -666,9 +669,12 @@ load_domaintype_info(TypeCacheEntry *typentry) /* * We try to optimize the common case of no domain constraints, so don't - * create the dcc object and context until we find a constraint. + * create the dcc object and context until we find a constraint. Likewise + * for the temp sorting array. */ dcc = NULL; + ccons = NULL; + cconslen = 0; /* * Scan pg_constraint for relevant constraints. We want to find @@ -682,6 +688,7 @@ load_domaintype_info(TypeCacheEntry *typentry) HeapTuple tup; HeapTuple conTup; Form_pg_type typTup; + int nccons = 0; ScanKeyData key[1]; SysScanDesc scan; @@ -763,17 +770,45 @@ load_domaintype_info(TypeCacheEntry *typentry) r->name = pstrdup(NameStr(c->conname)); r->check_expr = ExecInitExpr(check_expr, NULL); - /* - * Use lcons() here because constraints of parent domains should - * be applied earlier. - */ - dcc->constraints = lcons(r, dcc->constraints); - MemoryContextSwitchTo(oldcxt); + + /* Accumulate constraints in an array, for sorting below */ + if (ccons == NULL) + { + cconslen = 8; + ccons = (DomainConstraintState **) + palloc(cconslen * sizeof(DomainConstraintState *)); + } + else if (nccons >= cconslen) + { + cconslen *= 2; + ccons = (DomainConstraintState **) + repalloc(ccons, cconslen * sizeof(DomainConstraintState *)); + } + ccons[nccons++] = r; } systable_endscan(scan); + if (nccons > 0) + { + /* + * Sort the items for this domain, so that CHECKs are applied in a + * deterministic order. + */ + if (nccons > 1) + qsort(ccons, nccons, sizeof(DomainConstraintState *), dcs_cmp); + + /* + * Now attach them to the overall list. Use lcons() here because + * constraints of parent domains should be applied earlier. + */ + oldcxt = MemoryContextSwitchTo(dcc->dccContext); + while (nccons > 0) + dcc->constraints = lcons(ccons[--nccons], dcc->constraints); + MemoryContextSwitchTo(oldcxt); + } + /* loop to next domain in stack */ typeOid = typTup->typbasetype; ReleaseSysCache(tup); @@ -836,6 +871,18 @@ load_domaintype_info(TypeCacheEntry *typentry) typentry->flags |= TCFLAGS_CHECKED_DOMAIN_CONSTRAINTS; } +/* + * qsort comparator to sort DomainConstraintState pointers by name + */ +static int +dcs_cmp(const void *a, const void *b) +{ + const DomainConstraintState *const * ca = (const DomainConstraintState *const *) a; + const DomainConstraintState *const * cb = (const DomainConstraintState *const *) b; + + return strcmp((*ca)->name, (*cb)->name); +} + /* * decr_dcc_refcount --- decrement a DomainConstraintCache's refcount, * and free it if no references remain diff --git a/src/test/regress/input/constraints.source b/src/test/regress/input/constraints.source index 8ec00543fb..c16f65088a 100644 --- a/src/test/regress/input/constraints.source +++ b/src/test/regress/input/constraints.source @@ -87,7 +87,7 @@ CREATE SEQUENCE INSERT_SEQ; CREATE TABLE INSERT_TBL (x INT DEFAULT nextval('insert_seq'), y TEXT DEFAULT '-NULL-', z INT DEFAULT -1 * currval('insert_seq'), - CONSTRAINT INSERT_CON CHECK (x >= 3 AND y <> 'check failed' AND x < 8), + CONSTRAINT INSERT_TBL_CON CHECK (x >= 3 AND y <> 'check failed' AND x < 8), CHECK (x + z = 0)); INSERT INTO INSERT_TBL(x,z) VALUES (2, -2); diff --git a/src/test/regress/output/constraints.source b/src/test/regress/output/constraints.source index 0d32a9eab6..d3ec233313 100644 --- a/src/test/regress/output/constraints.source +++ b/src/test/regress/output/constraints.source @@ -116,10 +116,10 @@ CREATE SEQUENCE INSERT_SEQ; CREATE TABLE INSERT_TBL (x INT DEFAULT nextval('insert_seq'), y TEXT DEFAULT '-NULL-', z INT DEFAULT -1 * currval('insert_seq'), - CONSTRAINT INSERT_CON CHECK (x >= 3 AND y <> 'check failed' AND x < 8), + CONSTRAINT INSERT_TBL_CON CHECK (x >= 3 AND y <> 'check failed' AND x < 8), CHECK (x + z = 0)); INSERT INTO INSERT_TBL(x,z) VALUES (2, -2); -ERROR: new row for relation "insert_tbl" violates check constraint "insert_con" +ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con" DETAIL: Failing row contains (2, -NULL-, -2). SELECT '' AS zero, * FROM INSERT_TBL; zero | x | y | z @@ -133,7 +133,7 @@ SELECT 'one' AS one, nextval('insert_seq'); (1 row) INSERT INTO INSERT_TBL(y) VALUES ('Y'); -ERROR: new row for relation "insert_tbl" violates check constraint "insert_con" +ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con" DETAIL: Failing row contains (2, Y, -2). INSERT INTO INSERT_TBL(y) VALUES ('Y'); INSERT INTO INSERT_TBL(x,z) VALUES (1, -2); @@ -141,7 +141,7 @@ ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_ DETAIL: Failing row contains (1, -NULL-, -2). INSERT INTO INSERT_TBL(z,x) VALUES (-7, 7); INSERT INTO INSERT_TBL VALUES (5, 'check failed', -5); -ERROR: new row for relation "insert_tbl" violates check constraint "insert_con" +ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con" DETAIL: Failing row contains (5, check failed, -5). INSERT INTO INSERT_TBL VALUES (7, '!check failed', -7); INSERT INTO INSERT_TBL(y) VALUES ('-!NULL-'); @@ -158,7 +158,7 @@ INSERT INTO INSERT_TBL(y,z) VALUES ('check failed', 4); ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_check" DETAIL: Failing row contains (5, check failed, 4). INSERT INTO INSERT_TBL(x,y) VALUES (5, 'check failed'); -ERROR: new row for relation "insert_tbl" violates check constraint "insert_con" +ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con" DETAIL: Failing row contains (5, check failed, -5). INSERT INTO INSERT_TBL(x,y) VALUES (5, '!check failed'); INSERT INTO INSERT_TBL(y) VALUES ('-!NULL-'); @@ -180,7 +180,7 @@ SELECT 'seven' AS one, nextval('insert_seq'); (1 row) INSERT INTO INSERT_TBL(y) VALUES ('Y'); -ERROR: new row for relation "insert_tbl" violates check constraint "insert_con" +ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con" DETAIL: Failing row contains (8, Y, -8). SELECT 'eight' AS one, currval('insert_seq'); one | currval @@ -242,7 +242,7 @@ INSERT INTO INSERT_CHILD(x,z,cy) VALUES (6,-7,7); ERROR: new row for relation "insert_child" violates check constraint "insert_tbl_check" DETAIL: Failing row contains (6, -NULL-, -7, 42, 7). INSERT INTO INSERT_CHILD(x,y,z,cy) VALUES (6,'check failed',-6,7); -ERROR: new row for relation "insert_child" violates check constraint "insert_con" +ERROR: new row for relation "insert_child" violates check constraint "insert_tbl_con" DETAIL: Failing row contains (6, check failed, -6, 42, 7). SELECT * FROM INSERT_CHILD; x | y | z | cx | cy @@ -305,7 +305,7 @@ SELECT '' AS three, * FROM INSERT_TBL; INSERT INTO INSERT_TBL SELECT * FROM tmp WHERE yd = 'try again'; INSERT INTO INSERT_TBL(y,z) SELECT yd, -7 FROM tmp WHERE yd = 'try again'; INSERT INTO INSERT_TBL(y,z) SELECT yd, -8 FROM tmp WHERE yd = 'try again'; -ERROR: new row for relation "insert_tbl" violates check constraint "insert_con" +ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con" DETAIL: Failing row contains (8, try again, -8). SELECT '' AS four, * FROM INSERT_TBL; four | x | y | z @@ -325,7 +325,7 @@ UPDATE INSERT_TBL SET x = NULL WHERE x = 5; UPDATE INSERT_TBL SET x = 6 WHERE x = 6; UPDATE INSERT_TBL SET x = -z, z = -x; UPDATE INSERT_TBL SET x = z, z = x; -ERROR: new row for relation "insert_tbl" violates check constraint "insert_con" +ERROR: new row for relation "insert_tbl" violates check constraint "insert_tbl_con" DETAIL: Failing row contains (-4, Y, 4). SELECT * FROM INSERT_TBL; x | y | z