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

@@ -139,12 +139,8 @@ CREATE DOMAIN <replaceable class="parameter">name</replaceable> [ AS ] <replacea
       <term><literal>NOT NULL</></term>
       <listitem>
        <para>
-        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
-        <command>INSERT INTO tab (domcol) VALUES ((SELECT domcol FROM
-        tab WHERE false))</command>.
+        Values of this domain are prevented from being null
+        (but see notes below).
        </para>
       </listitem>
      </varlistentry>
@@ -171,18 +167,55 @@ CREATE DOMAIN <replaceable class="parameter">name</replaceable> [ AS ] <replacea
       which values of the domain must satisfy.
       Each constraint must be an expression
       producing a Boolean result.  It should use the key word <literal>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.
      </para>
 
      <para>
       Currently, <literal>CHECK</literal> expressions cannot contain
       subqueries nor refer to variables other than <literal>VALUE</>.
      </para>
+
+     <para>
+      When a domain has multiple <literal>CHECK</literal> constraints,
+      they will be tested in alphabetical order by name.
+      (<productname>PostgreSQL</> versions before 9.5 did not honor any
+      particular firing order for <literal>CHECK</literal> constraints.)
+     </para>
     </listitem>
    </varlistentry>
   </variablelist>
  </refsect1>
 
+ <refsect1>
+  <title>Notes</title>
+
+  <para>
+   Domain constraints, particularly <literal>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
+<programlisting>
+INSERT INTO tab (domcol) VALUES ((SELECT domcol FROM tab WHERE false));
+</programlisting>
+   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.
+  </para>
+
+  <para>
+   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 <literal>NOT NULL</> constraints to columns of
+   the domain type as needed, rather than directly to the domain type.
+  </para>
+ </refsect1>
+
  <refsect1>
   <title>Examples</title>
 

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 <literal>NO INHERIT</> will not propagate to
       child tables.
      </para>
+
+     <para>
+      When a table has multiple <literal>CHECK</literal> constraints,
+      they will be tested for each row in alphabetical order by name,
+      after checking <literal>NOT NULL</> constraints.
+      (<productname>PostgreSQL</> versions before 9.5 did not honor any
+      particular firing order for <literal>CHECK</literal> constraints.)
+     </para>
     </listitem>
    </varlistentry>
 

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);
 }
 
 /*

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);
 
+			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)
+		{
 			/*
-			 * Use lcons() here because constraints of parent domains should
-			 * be applied earlier.
+			 * Sort the items for this domain, so that CHECKs are applied in a
+			 * deterministic order.
 			 */
-			dcc->constraints = lcons(r, dcc->constraints);
+			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);
 		}
 
-		systable_endscan(scan);
-
 		/* 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

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);

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