Fix ExecSubPlan to handle nulls per the SQL spec --- it didn't combine

nulls with non-nulls using proper three-valued boolean logic.  Also clean
up ExecQual to make it clearer that ExecQual *does* follow the SQL spec
for boolean nulls.  See '[BUGS] (null) != (null)' thread around 10/26/99
for more detail.

src/backend/executor/execQual.c

@@ -7,14 +7,14 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.63 1999/10/08 03:49:55 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/executor/execQual.c,v 1.64 1999/11/12 06:39:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 /*
  *	 INTERFACE ROUTINES
  *		ExecEvalExpr	- evaluate an expression and return a datum
- *		ExecQual		- return true/false if qualification is satisified
+ *		ExecQual		- return true/false if qualification is satisfied
  *		ExecTargetList	- form a new tuple by projecting the given tuple
  *
  *	 NOTES
@@ -71,7 +71,6 @@ static Datum ExecEvalOr(Expr *orExpr, ExprContext *econtext, bool *isNull);
 static Datum ExecEvalVar(Var *variable, ExprContext *econtext, bool *isNull);
 static Datum ExecMakeFunctionResult(Node *node, List *arguments,
 					   ExprContext *econtext, bool *isNull, bool *isDone);
-static bool ExecQualClause(Node *clause, ExprContext *econtext);
 
 /*
  *	  ExecEvalArrayRef
@@ -1253,7 +1252,9 @@ ExecEvalExpr(Node *expression,
 						retDatum = (Datum) ExecEvalNot(expr, econtext, isNull);
 						break;
 					case SUBPLAN_EXPR:
-						retDatum = (Datum) ExecSubPlan((SubPlan *) expr->oper, expr->args, econtext);
+						retDatum = (Datum) ExecSubPlan((SubPlan *) expr->oper,
+													   expr->args, econtext,
+													   isNull);
 						break;
 					default:
 						elog(ERROR, "ExecEvalExpr: unknown expression type %d", expr->opType);
@@ -1279,46 +1280,6 @@ ExecEvalExpr(Node *expression,
  * ----------------------------------------------------------------
  */
 
-/* ----------------------------------------------------------------
- *		ExecQualClause
- *
- *		this is a workhorse for ExecQual.  ExecQual has to deal
- *		with a list of qualifications, so it passes each qualification
- *		in the list to this function one at a time.  ExecQualClause
- *		returns true when the qualification *fails* and false if
- *		the qualification succeeded (meaning we have to test the
- *		rest of the qualification)
- * ----------------------------------------------------------------
- */
-static bool
-ExecQualClause(Node *clause, ExprContext *econtext)
-{
-	Datum		expr_value;
-	bool		isNull;
-	bool		isDone;
-
-	/* when there is a null clause, consider the qualification to fail */
-	if (clause == NULL)
-		return true;
-
-	/*
-	 * pass isDone, but ignore it.	We don't iterate over multiple returns
-	 * in the qualifications.
-	 */
-	expr_value = ExecEvalExpr(clause, econtext, &isNull, &isDone);
-
-	/*
-	 * remember, we return true when the qualification fails;
-	 * NULL is considered failure.
-	 */
-	if (isNull)
-		return true;
-	if (DatumGetInt32(expr_value) == 0)
-		return true;
-
-	return false;
-}
-
 /* ----------------------------------------------------------------
  *		ExecQual
  *
@@ -1329,7 +1290,7 @@ ExecQualClause(Node *clause, ExprContext *econtext)
 bool
 ExecQual(List *qual, ExprContext *econtext)
 {
-	List	   *clause;
+	List	   *qlist;
 
 	/*
 	 * debugging stuff
@@ -1340,25 +1301,38 @@ ExecQual(List *qual, ExprContext *econtext)
 
 	IncrProcessed();
 
-	/*
-	 * return true immediately if no qual
-	 */
-	if (qual == NIL)
-		return true;
-
 	/*
 	 * a "qual" is a list of clauses.  To evaluate the qual, we evaluate
-	 * each of the clauses in the list.
+	 * each of the clauses in the list.  (For an empty list, we'll return
+	 * TRUE.)
 	 *
-	 * ExecQualClause returns true when we know the qualification *failed*
-	 * so we just pass each clause in qual to it until we know the qual
-	 * failed or there are no more clauses.
+	 * 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.
 	 */
-
-	foreach(clause, qual)
+	foreach(qlist, qual)
 	{
-		if (ExecQualClause((Node *) lfirst(clause), econtext))
-			return false;		/* qual failed, so return false */
+		Node	   *clause = (Node *) lfirst(qlist);
+		Datum		expr_value;
+		bool		isNull;
+		bool		isDone;
+
+		/* if there is a null clause, consider the qualification to fail */
+		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;
 	}
 
 	return true;

src/backend/executor/nodeSubplan.c

@@ -18,34 +18,39 @@
 #include "executor/nodeSubplan.h"
 #include "tcop/pquery.h"
 
+/* should be exported by execMain.c */
+extern void ExecCheckPerms(CmdType op, int resRel, List *rtable, Query *q);
+
 /* ----------------------------------------------------------------
  *		ExecSubPlan(node)
  *
  * ----------------------------------------------------------------
  */
 Datum
-ExecSubPlan(SubPlan *node, List *pvar, ExprContext *econtext)
+ExecSubPlan(SubPlan *node, List *pvar, ExprContext *econtext, bool *isNull)
 {
 	Plan	   *plan = node->plan;
 	SubLink    *sublink = node->sublink;
 	SubLinkType subLinkType = sublink->subLinkType;
+	bool		useor = sublink->useor;
 	TupleTableSlot *slot;
-	List	   *lst;
-	Datum		result = (Datum) false;
+	Datum		result;
 	bool		found = false;	/* TRUE if got at least one subplan tuple */
+	List	   *lst;
 
-	if (node->setParam != NULL)
+	if (node->setParam != NIL)
 		elog(ERROR, "ExecSubPlan: can't set parent params from subquery");
 
 	/*
 	 * Set Params of this plan from parent plan correlation Vars
 	 */
-	if (node->parParam != NULL)
+	if (node->parParam != NIL)
 	{
 		foreach(lst, node->parParam)
 		{
 			ParamExecData *prm = &(econtext->ecxt_param_exec_vals[lfirsti(lst)]);
 
+			Assert(pvar != NIL);
 			prm->value = ExecEvalExpr((Node *) lfirst(pvar),
 									  econtext,
 									  &(prm->isnull), NULL);
@@ -53,21 +58,32 @@ ExecSubPlan(SubPlan *node, List *pvar, ExprContext *econtext)
 		}
 		plan->chgParam = nconc(plan->chgParam, listCopy(node->parParam));
 	}
+	Assert(pvar == NIL);
 
 	ExecReScan(plan, (ExprContext *) NULL, plan);
 
 	/*
-	 * For all sublink types except EXPR_SUBLINK, the result type is
-	 * boolean, and we have a fairly clear idea of how to combine multiple
-	 * subitems and deal with NULL values or an empty subplan result.
+	 * For all sublink types except EXPR_SUBLINK, the result is boolean
+	 * as are the results of the combining operators.  We combine results
+	 * within a tuple (if there are multiple columns) using OR semantics
+	 * if "useor" is true, AND semantics if not.  We then combine results
+	 * across tuples (if the subplan produces more than one) using OR
+	 * semantics for ANY_SUBLINK or AND semantics for ALL_SUBLINK.  NULL
+	 * results from the combining operators are handled according to the
+	 * usual SQL semantics for OR and AND.  The result for no input
+	 * tuples is FALSE for ANY_SUBLINK, TRUE for ALL_SUBLINK.
 	 *
-	 * For EXPR_SUBLINK, the result type is whatever the combining operator
-	 * returns.  We have no way to deal with more than one column in the
-	 * subplan result --- hopefully the parser forbids that.  More
-	 * seriously, it's unclear what to do with NULL values or an empty
-	 * subplan result. For now, we error out, but should something else
-	 * happen?
+	 * For EXPR_SUBLINK we require the subplan to produce no more than one
+	 * tuple, else an error is raised.  If zero tuples are produced, we
+	 * return NULL.  (XXX it would probably be more correct to evaluate
+	 * the combining operator with a NULL input?)  Assuming we get a tuple:
+	 * if there is only one column then we just return its result as-is, NULL
+	 * or otherwise.  If there is more than one column we combine the results
+	 * per "useor" --- this only makes sense if the combining operators yield
+	 * boolean, and we assume the parser has checked that.
 	 */
+	result = (Datum) (subLinkType == ALL_SUBLINK ? true : false);
+	*isNull = false;
 
 	for (slot = ExecProcNode(plan, plan);
 		 !TupIsNull(slot);
@@ -75,24 +91,26 @@ ExecSubPlan(SubPlan *node, List *pvar, ExprContext *econtext)
 	{
 		HeapTuple	tup = slot->val;
 		TupleDesc	tdesc = slot->ttc_tupleDescriptor;
-		int			i = 1;
-
-		if (subLinkType == EXPR_SUBLINK && found)
-		{
-			elog(ERROR, "ExecSubPlan: more than one tuple returned by expression subselect");
-			return (Datum) false;
-		}
+		Datum		rowresult = (Datum) (useor ? false : true);
+		bool		rownull = false;
+		int			col = 1;
 
 		if (subLinkType == EXISTS_SUBLINK)
 			return (Datum) true;
 
+		/* cannot allow multiple input tuples for EXPR sublink */
+		if (subLinkType == EXPR_SUBLINK && found)
+			elog(ERROR, "ExecSubPlan: more than one tuple returned by expression subselect");
+
 		found = true;
 
+		/* iterate over combining operators for columns of tuple */
 		foreach(lst, sublink->oper)
 		{
 			Expr	   *expr = (Expr *) lfirst(lst);
 			Const	   *con = lsecond(expr->args);
-			bool		isnull;
+			Datum		expresult;
+			bool		expnull;
 
 			/*
 			 * The righthand side of the expression should be either a Const
@@ -107,41 +125,90 @@ ExecSubPlan(SubPlan *node, List *pvar, ExprContext *econtext)
 				con = lfirst(((Expr *) con)->args);
 				Assert(IsA(con, Const));
 			}
-			con->constvalue = heap_getattr(tup, i, tdesc, &(con->constisnull));
+			con->constvalue = heap_getattr(tup, col, tdesc,
+										   &(con->constisnull));
 			/*
-			 * Now we can eval the expression.
+			 * Now we can eval the combining operator for this column.
 			 */
-			result = ExecEvalExpr((Node *) expr, econtext, &isnull,
-								  (bool *) NULL);
-			if (isnull)
+			expresult = ExecEvalExpr((Node *) expr, econtext, &expnull,
+									 (bool *) NULL);
+			/*
+			 * Combine the result into the row result as appropriate.
+			 */
+			if (col == 1)
 			{
-				if (subLinkType == EXPR_SUBLINK)
-					elog(ERROR, "ExecSubPlan: null value returned by expression subselect");
-				else
-					result = (Datum) false;
+				rowresult = expresult;
+				rownull = expnull;
 			}
-			if (subLinkType != EXPR_SUBLINK)
+			else if (useor)
 			{
-				if ((!(bool) result && !(sublink->useor)) ||
-					((bool) result && sublink->useor))
-					break;
+				/* combine within row per OR semantics */
+				if (expnull)
+					rownull = true;
+				else if (DatumGetInt32(expresult) != 0)
+				{
+					rowresult = (Datum) true;
+					rownull = false;
+					break;		/* needn't look at any more columns */
+				}
 			}
-			i++;
+			else
+			{
+				/* combine within row per AND semantics */
+				if (expnull)
+					rownull = true;
+				else if (DatumGetInt32(expresult) == 0)
+				{
+					rowresult = (Datum) false;
+					rownull = false;
+					break;		/* needn't look at any more columns */
+				}
+			}
+			col++;
 		}
 
-		if (subLinkType == ALL_SUBLINK && !(bool) result)
-			break;
-		if (subLinkType == ANY_SUBLINK && (bool) result)
-			break;
+		if (subLinkType == ANY_SUBLINK)
+		{
+			/* combine across rows per OR semantics */
+			if (rownull)
+				*isNull = true;
+			else if (DatumGetInt32(rowresult) != 0)
+			{
+				result = (Datum) true;
+				*isNull = false;
+				break;			/* needn't look at any more rows */
+			}
+		}
+		else if (subLinkType == ALL_SUBLINK)
+		{
+			/* combine across rows per AND semantics */
+			if (rownull)
+				*isNull = true;
+			else if (DatumGetInt32(rowresult) == 0)
+			{
+				result = (Datum) false;
+				*isNull = false;
+				break;			/* needn't look at any more rows */
+			}
+		}
+		else
+		{
+			/* must be EXPR_SUBLINK */
+			result = rowresult;
+			*isNull = rownull;
+		}
 	}
 
 	if (!found)
 	{
-		/* deal with empty subplan result.	Note default result is 'false' */
-		if (subLinkType == ALL_SUBLINK)
-			result = (Datum) true;
-		else if (subLinkType == EXPR_SUBLINK)
-			elog(ERROR, "ExecSubPlan: no tuples returned by expression subselect");
+		/* deal with empty subplan result.  result/isNull were previously
+		 * initialized correctly for all sublink types except EXPR.
+		 */
+		if (subLinkType == EXPR_SUBLINK)
+		{
+				result = (Datum) false;
+				*isNull = true;
+		}
 	}
 
 	return result;
@@ -152,7 +219,6 @@ ExecSubPlan(SubPlan *node, List *pvar, ExprContext *econtext)
  *
  * ----------------------------------------------------------------
  */
-extern void ExecCheckPerms(CmdType op, int resRel, List *rtable, Query *q);
 bool
 ExecInitSubPlan(SubPlan *node, EState *estate, Plan *parent)
 {

src/include/executor/nodeSubplan.h

@@ -9,7 +9,8 @@
 
 #include "nodes/plannodes.h"
 
-extern Datum ExecSubPlan(SubPlan *node, List *pvar, ExprContext *econtext);
+extern Datum ExecSubPlan(SubPlan *node, List *pvar, ExprContext *econtext,
+						 bool *isNull);
 extern bool ExecInitSubPlan(SubPlan *node, EState *estate, Plan *parent);
 extern void ExecReScanSetParamPlan(SubPlan *node, Plan *parent);
 extern void ExecSetParamPlan(SubPlan *node);