Fix handling of NULL constraint conditions: per SQL92 spec, a NULL result

from a constraint condition does not violate the constraint (cf. discussion
on pghackers 12/9/99).  Implemented by adding a parameter to ExecQual,
specifying whether to return TRUE or FALSE when the qual result is
really NULL in three-valued boolean logic.  Currently, ExecRelCheck is
the only caller that asks for TRUE, but if we find any other places that
have the wrong response to NULL, it'll be easy to fix them.

Author: tglsfdc

src/backend/access/gist/gist.c

@@ -6,7 +6,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/gist/gist.c,v 1.49 2000/01/17 23:57:41 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/gist/gist.c,v 1.50 2000/01/19 23:54:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -169,7 +169,7 @@ gistbuild(Relation heap,
 #ifndef OMIT_PARTIAL_INDEX
 			/* SetSlotContents(slot, htup); */
 			slot->val = htup;
-			if (ExecQual((List *) oldPred, econtext) == true)
+			if (ExecQual((List *) oldPred, econtext, false))
 			{
 				ni++;
 				continue;
@@ -186,7 +186,7 @@ gistbuild(Relation heap,
 #ifndef OMIT_PARTIAL_INDEX
 			/* SetSlotContents(slot, htup); */
 			slot->val = htup;
-			if (ExecQual((List *) pred, econtext) == false)
+			if (! ExecQual((List *) pred, econtext, false))
 				continue;
 #endif	 /* OMIT_PARTIAL_INDEX */
 		}

src/backend/access/hash/hash.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/hash/hash.c,v 1.33 1999/12/10 03:55:43 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/hash/hash.c,v 1.34 2000/01/19 23:54:47 tgl Exp $
  *
  * NOTES
  *	  This file contains only the public interface routines.
@@ -131,7 +131,7 @@ hashbuild(Relation heap,
 			/* SetSlotContents(slot, htup); */
 #ifndef OMIT_PARTIAL_INDEX
 			slot->val = htup;
-			if (ExecQual((List *) oldPred, econtext) == true)
+			if (ExecQual((List *) oldPred, econtext, false))
 			{
 				nitups++;
 				continue;
@@ -148,7 +148,7 @@ hashbuild(Relation heap,
 #ifndef OMIT_PARTIAL_INDEX
 			/* SetSlotContents(slot, htup); */
 			slot->val = htup;
-			if (ExecQual((List *) pred, econtext) == false)
+			if (! ExecQual((List *) pred, econtext, false))
 				continue;
 #endif	 /* OMIT_PARTIAL_INDEX */
 		}

src/backend/access/nbtree/nbtree.c

@@ -11,7 +11,7 @@
  * Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtree.c,v 1.50 1999/12/10 03:55:44 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/nbtree/nbtree.c,v 1.51 2000/01/19 23:54:48 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -167,7 +167,7 @@ btbuild(Relation heap,
 
 			/* SetSlotContents(slot, htup); */
 			slot->val = htup;
-			if (ExecQual((List *) oldPred, econtext) == true)
+			if (ExecQual((List *) oldPred, econtext, false))
 			{
 				nitups++;
 				continue;
@@ -184,7 +184,7 @@ btbuild(Relation heap,
 #ifndef OMIT_PARTIAL_INDEX
 			/* SetSlotContents(slot, htup); */
 			slot->val = htup;
-			if (ExecQual((List *) pred, econtext) == false)
+			if (! ExecQual((List *) pred, econtext, false))
 				continue;
 #endif	 /* OMIT_PARTIAL_INDEX */
 		}

src/backend/access/rtree/rtree.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/rtree/Attic/rtree.c,v 1.41 1999/12/10 03:55:45 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/rtree/Attic/rtree.c,v 1.42 2000/01/19 23:54:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -163,7 +163,7 @@ rtbuild(Relation heap,
 #ifndef OMIT_PARTIAL_INDEX
 			/* SetSlotContents(slot, htup); */
 			slot->val = htup;
-			if (ExecQual((List *) oldPred, econtext) == true)
+			if (ExecQual((List *) oldPred, econtext, false))
 			{
 				ni++;
 				continue;
@@ -180,7 +180,7 @@ rtbuild(Relation heap,
 #ifndef OMIT_PARTIAL_INDEX
 			/* SetSlotContents(slot, htup); */
 			slot->val = htup;
-			if (ExecQual((List *) pred, econtext) == false)
+			if (! ExecQual((List *) pred, econtext, false))
 				continue;
 #endif	 /* OMIT_PARTIAL_INDEX */
 		}

src/backend/catalog/index.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.102 2000/01/17 23:57:43 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.103 2000/01/19 23:54:51 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1590,7 +1590,7 @@ DefaultBuild(Relation heapRelation,
 		{
 			/* SetSlotContents(slot, heapTuple); */
 			slot->val = heapTuple;
-			if (ExecQual((List *) oldPred, econtext) == true)
+			if (ExecQual((List *) oldPred, econtext, false))
 			{
 				indtuples++;
 				continue;
@@ -1605,7 +1605,7 @@ DefaultBuild(Relation heapRelation,
 		{
 			/* SetSlotContents(slot, heapTuple); */
 			slot->val = heapTuple;
-			if (ExecQual((List *) predicate, econtext) == false)
+			if (! ExecQual((List *) predicate, econtext, false))
 				continue;
 		}
 #endif	 /* OMIT_PARTIAL_INDEX */

src/backend/commands/copy.c

@@ -6,7 +6,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.96 2000/01/16 21:37:50 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.97 2000/01/19 23:54:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -886,7 +886,7 @@ CopyFrom(Relation rel, bool binary, bool oids, FILE *fp, char *delim, char *null
 						 */
 						slot->val = tuple;
 						/* SetSlotContents(slot, tuple); */
-						if (ExecQual((List *) indexPred[i], econtext) == false)
+						if (! ExecQual((List *) indexPred[i], econtext, false))
 							continue;
 #endif	 /* OMIT_PARTIAL_INDEX */
 					}

src/backend/executor/execMain.c

@@ -26,7 +26,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.105 2000/01/17 23:57:45 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.106 2000/01/19 23:54:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1487,7 +1487,6 @@ ExecRelCheck(Relation rel, HeapTuple tuple, EState *estate)
 	RangeTblEntry *rte = makeNode(RangeTblEntry);
 	List	   *rtlist;
 	List	   *qual;
-	bool		res;
 	int			i;
 
 	slot->val = tuple;
@@ -1526,9 +1525,12 @@ ExecRelCheck(Relation rel, HeapTuple tuple, EState *estate)
 	{
 		qual = estate->es_result_relation_constraints[i];
 
-		res = ExecQual(qual, econtext);
-
-		if (!res)
+		/*
+		 * NOTE: SQL92 specifies that a NULL result from a constraint
+		 * expression is not to be treated as a failure.  Therefore,
+		 * tell ExecQual to return TRUE for NULL.
+		 */
+		if (! ExecQual(qual, econtext, true))
 			return check[i].ccname;
 	}
 

src/backend/executor/execQual.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.65 2000/01/10 17:14:34 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.66 2000/01/19 23:54:54 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1283,12 +1283,33 @@ ExecEvalExpr(Node *expression,
 /* ----------------------------------------------------------------
  *		ExecQual
  *
- *		Evaluates a conjunctive boolean expression and returns t
- *		iff none of the subexpressions are false (or null).
+ *		Evaluates a conjunctive boolean expression (qual list) and
+ *		returns true iff none of the subexpressions are false.
+ *		(We also return true if the list is empty.)
+ *
+ *	If some of the subexpressions yield NULL but none yield FALSE,
+ *	then the result of the conjunction is NULL (ie, unknown)
+ *	according to three-valued boolean logic.  In this case,
+ *	we return the value specified by the "resultForNull" parameter.
+ *
+ *	Callers evaluating WHERE clauses should pass resultForNull=FALSE,
+ *	since SQL specifies that tuples with null WHERE results do not
+ *	get selected.  On the other hand, callers evaluating constraint
+ *	conditions should pass resultForNull=TRUE, since SQL also specifies
+ *	that NULL constraint conditions are not failures.
+ *
+ *	NOTE: it would not be correct to use this routine to evaluate an
+ *	AND subclause of a boolean expression; for that purpose, a NULL
+ *	result must be returned as NULL so that it can be properly treated
+ *	in the next higher operator (cf. ExecEvalAnd and ExecEvalOr).
+ *	This routine is only used in contexts where a complete expression
+ *	is being evaluated and we know that NULL can be treated the same
+ *	as one boolean result or the other.
+ *
  * ----------------------------------------------------------------
  */
 bool
-ExecQual(List *qual, ExprContext *econtext)
+ExecQual(List *qual, ExprContext *econtext, bool resultForNull)
 {
 	List	   *qlist;
 
@@ -1302,37 +1323,48 @@ ExecQual(List *qual, ExprContext *econtext)
 	IncrProcessed();
 
 	/*
-	 * a "qual" is a list of clauses.  To evaluate the qual, we evaluate
-	 * each of the clauses in the list.  (For an empty list, we'll return
-	 * TRUE.)
+	 * Evaluate the qual conditions one at a time.  If we find a FALSE
+	 * result, we can stop evaluating and return FALSE --- the AND result
+	 * must be FALSE.  Also, if we find a NULL result when resultForNull
+	 * is FALSE, we can stop and return FALSE --- the AND result must be
+	 * FALSE or NULL in that case, and the caller doesn't care which.
 	 *
-	 * If any of the clauses return NULL, we treat this as FALSE.  This
-	 * is correct per the SQL spec: if any ANDed conditions are NULL, then
-	 * the AND result is either FALSE or NULL, and in either case the
-	 * WHERE condition fails.  NOTE: it would NOT be correct to use this
-	 * simplified logic in a sub-clause; ExecEvalAnd must do the full
-	 * three-state condition evaluation.  We can get away with simpler
-	 * logic here because we know how the result will be used.
+	 * If we get to the end of the list, we can return TRUE.  This will
+	 * happen when the AND result is indeed TRUE, or when the AND result
+	 * is NULL (one or more NULL subresult, with all the rest TRUE) and
+	 * the caller has specified resultForNull = TRUE.
 	 */
+
 	foreach(qlist, qual)
 	{
 		Node	   *clause = (Node *) lfirst(qlist);
 		Datum		expr_value;
 		bool		isNull;
 		bool		isDone;
 
-		/* if there is a null clause, consider the qualification to fail */
+		/*
+		 * If there is a null clause, consider the qualification to fail.
+		 * XXX is this still correct for constraints?  It probably shouldn't
+		 * happen at all ...
+		 */
 		if (clause == NULL)
 			return false;
 		/*
 		 * pass isDone, but ignore it.	We don't iterate over multiple returns
 		 * in the qualifications.
 		 */
 		expr_value = ExecEvalExpr(clause, econtext, &isNull, &isDone);
+
 		if (isNull)
-			return false;		/* treat NULL as FALSE */
-		if (DatumGetInt32(expr_value) == 0)
-			return false;
+		{
+			if (resultForNull == false)
+				return false;	/* treat NULL as FALSE */
+		}
+		else
+		{
+			if (DatumGetInt32(expr_value) == 0)
+				return false;	/* definitely FALSE */
+		}
 	}
 
 	return true;

src/backend/executor/execScan.c

@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execScan.c,v 1.9 1999/02/13 23:15:18 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execScan.c,v 1.10 2000/01/19 23:54:54 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -137,9 +137,10 @@ ExecScan(Scan *node,
 
 		/*
 		 * add a check for non-nil qual here to avoid a function call to
-		 * ExecQual() when the qual is nil
+		 * ExecQual() when the qual is nil ... saves only a few cycles,
+		 * but they add up ...
 		 */
-		if (!qual || ExecQual(qual, econtext) == true)
+		if (!qual || ExecQual(qual, econtext, false))
 			break;
 	}
 

src/backend/executor/execUtils.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execUtils.c,v 1.51 1999/12/20 10:40:42 wieck Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execUtils.c,v 1.52 2000/01/19 23:54:54 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1137,7 +1137,6 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
 	IndexInfo **indexInfoArray;
 	IndexInfo  *indexInfo;
 	Node	   *predicate;
-	bool		satisfied;
 	ExprContext *econtext;
 	InsertIndexResult result;
 	int			numberOfAttributes;
@@ -1178,8 +1177,7 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
 			econtext->ecxt_scantuple = slot;
 
 			/* Skip this index-update if the predicate isn't satisfied */
-			satisfied = ExecQual((List *) predicate, econtext);
-			if (satisfied == false)
+			if (! ExecQual((List *) predicate, econtext, false))
 				continue;
 		}
 

src/backend/executor/nodeAgg.c

@@ -31,7 +31,7 @@
  * Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeAgg.c,v 1.60 1999/12/13 01:26:52 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/nodeAgg.c,v 1.61 2000/01/19 23:54:54 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -580,7 +580,7 @@ ExecAgg(Agg *node)
 		 * Otherwise, return the tuple.
 		 */
 	}
-	while (! ExecQual(node->plan.qual, econtext));
+	while (! ExecQual(node->plan.qual, econtext, false));
 
 	return resultSlot;
 }

src/backend/executor/nodeHash.c

@@ -6,7 +6,7 @@
  * Copyright (c) 1994, Regents of the University of California
  *
  *
- *	$Id: nodeHash.c,v 1.42 2000/01/09 00:26:18 tgl Exp $
+ *	$Id: nodeHash.c,v 1.43 2000/01/19 23:54:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -584,7 +584,6 @@ ExecScanHashBucket(HashJoinState *hjstate,
 	{
 		HeapTuple	heapTuple = &hashTuple->htup;
 		TupleTableSlot *inntuple;
-		bool		qualResult;
 
 		/* insert hashtable's tuple into exec slot so ExecQual sees it */
 		inntuple = ExecStoreTuple(heapTuple,	/* tuple to store */
@@ -593,9 +592,7 @@ ExecScanHashBucket(HashJoinState *hjstate,
 								  false);		/* do not pfree this tuple */
 		econtext->ecxt_innertuple = inntuple;
 
-		qualResult = ExecQual(hjclauses, econtext);
-
-		if (qualResult)
+		if (ExecQual(hjclauses, econtext, false))
 		{
 			hjstate->hj_CurTuple = hashTuple;
 			return heapTuple;