Fix IS NULL and IS NOT NULL tests on row-valued expressions to conform to

the SQL spec, viz IS NULL is true if all the row's fields are null, IS NOT
NULL is true if all the row's fields are not null.  The former coding got
this right for a limited number of cases with IS NULL (ie, those where it
could disassemble a ROW constructor at parse time), but was entirely wrong
for IS NOT NULL.  Per report from Teodor.

I desisted from changing the behavior for arrays, since on closer inspection
it's not clear that there's any support for that in the SQL spec.  This
probably needs more consideration.

Author: tglsfdc

doc/src/sgml/func.sgml

@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.340 2006/09/22 16:20:00 tgl Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.341 2006/09/28 20:51:41 tgl Exp $ -->
 
  <chapter id="functions">
   <title>Functions and Operators</title>
@@ -338,6 +338,19 @@
    </para>
   </tip>
 
+  <note>
+   <para>
+    If the <replaceable>expression</replaceable> is row-valued, then
+    <literal>IS NULL</> is true when the row expression itself is null
+    or when all the row's fields are null, while
+    <literal>IS NOT NULL</> is true when the row expression itself is non-null
+    and all the row's fields are non-null.
+    This definition conforms to the SQL standard, and is a change from the
+    inconsistent behavior exhibited by <productname>PostgreSQL</productname>
+    versions prior to 8.2.
+   </para>
+  </note>
+
    <para>
     <indexterm>
      <primary>IS DISTINCT FROM</primary>

src/backend/executor/execQual.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.193 2006/07/27 19:52:05 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.194 2006/09/28 20:51:41 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -119,7 +119,7 @@ static Datum ExecEvalMinMax(MinMaxExprState *minmaxExpr,
 static Datum ExecEvalNullIf(FuncExprState *nullIfExpr,
 			   ExprContext *econtext,
 			   bool *isNull, ExprDoneCond *isDone);
-static Datum ExecEvalNullTest(GenericExprState *nstate,
+static Datum ExecEvalNullTest(NullTestState *nstate,
 				 ExprContext *econtext,
 				 bool *isNull, ExprDoneCond *isDone);
 static Datum ExecEvalBooleanTest(GenericExprState *bstate,
@@ -1247,8 +1247,7 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
 
 	funcrettype = exprType((Node *) funcexpr->expr);
 
-	returnsTuple = (funcrettype == RECORDOID ||
-					get_typtype(funcrettype) == 'c');
+	returnsTuple = type_is_rowtype(funcrettype);
 
 	/*
 	 * Prepare a resultinfo node for communication.  We always do this even if
@@ -2683,7 +2682,7 @@ ExecEvalNullIf(FuncExprState *nullIfExpr,
  * ----------------------------------------------------------------
  */
 static Datum
-ExecEvalNullTest(GenericExprState *nstate,
+ExecEvalNullTest(NullTestState *nstate,
 				 ExprContext *econtext,
 				 bool *isNull,
 				 ExprDoneCond *isDone)
@@ -2696,28 +2695,77 @@ ExecEvalNullTest(GenericExprState *nstate,
 	if (isDone && *isDone == ExprEndResult)
 		return result;			/* nothing to check */
 
-	switch (ntest->nulltesttype)
+	if (nstate->argisrow && !(*isNull))
 	{
-		case IS_NULL:
-			if (*isNull)
+		HeapTupleHeader tuple;
+		Oid			tupType;
+		int32		tupTypmod;
+		TupleDesc	tupDesc;
+		HeapTupleData tmptup;
+		int			att;
+
+		tuple = DatumGetHeapTupleHeader(result);
+
+		tupType = HeapTupleHeaderGetTypeId(tuple);
+		tupTypmod = HeapTupleHeaderGetTypMod(tuple);
+
+		/* Lookup tupdesc if first time through or if type changes */
+		tupDesc = get_cached_rowtype(tupType, tupTypmod,
+									 &nstate->argdesc, econtext);
+
+		/*
+		 * heap_attisnull needs a HeapTuple not a bare HeapTupleHeader.
+		 */
+		tmptup.t_len = HeapTupleHeaderGetDatumLength(tuple);
+		tmptup.t_data = tuple;
+
+		for (att = 1; att <= tupDesc->natts; att++)
+		{
+			/* ignore dropped columns */
+			if (tupDesc->attrs[att-1]->attisdropped)
+				continue;
+			if (heap_attisnull(&tmptup, att))
 			{
-				*isNull = false;
-				return BoolGetDatum(true);
+				/* null field disproves IS NOT NULL */
+				if (ntest->nulltesttype == IS_NOT_NULL)
+					return BoolGetDatum(false);
 			}
 			else
-				return BoolGetDatum(false);
-		case IS_NOT_NULL:
-			if (*isNull)
 			{
-				*isNull = false;
-				return BoolGetDatum(false);
+				/* non-null field disproves IS NULL */
+				if (ntest->nulltesttype == IS_NULL)
+					return BoolGetDatum(false);
 			}
-			else
-				return BoolGetDatum(true);
-		default:
-			elog(ERROR, "unrecognized nulltesttype: %d",
-				 (int) ntest->nulltesttype);
-			return (Datum) 0;	/* keep compiler quiet */
+		}
+
+		return BoolGetDatum(true);
+	}
+	else
+	{
+		/* Simple scalar-argument case, or a null rowtype datum */
+		switch (ntest->nulltesttype)
+		{
+			case IS_NULL:
+				if (*isNull)
+				{
+					*isNull = false;
+					return BoolGetDatum(true);
+				}
+				else
+					return BoolGetDatum(false);
+			case IS_NOT_NULL:
+				if (*isNull)
+				{
+					*isNull = false;
+					return BoolGetDatum(false);
+				}
+				else
+					return BoolGetDatum(true);
+			default:
+				elog(ERROR, "unrecognized nulltesttype: %d",
+					 (int) ntest->nulltesttype);
+				return (Datum) 0;	/* keep compiler quiet */
+		}
 	}
 }
 
@@ -3609,11 +3657,13 @@ ExecInitExpr(Expr *node, PlanState *parent)
 		case T_NullTest:
 			{
 				NullTest   *ntest = (NullTest *) node;
-				GenericExprState *gstate = makeNode(GenericExprState);
+				NullTestState *nstate = makeNode(NullTestState);
 
-				gstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalNullTest;
-				gstate->arg = ExecInitExpr(ntest->arg, parent);
-				state = (ExprState *) gstate;
+				nstate->xprstate.evalfunc = (ExprStateEvalFunc) ExecEvalNullTest;
+				nstate->arg = ExecInitExpr(ntest->arg, parent);
+				nstate->argisrow = type_is_rowtype(exprType((Node *) ntest->arg));
+				nstate->argdesc = NULL;
+				state = (ExprState *) nstate;
 			}
 			break;
 		case T_BooleanTest:

src/backend/optimizer/util/clauses.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.220 2006/09/06 20:40:47 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.221 2006/09/28 20:51:41 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -2099,6 +2099,85 @@ eval_const_expressions_mutator(Node *node,
 		newfselect->resulttypmod = fselect->resulttypmod;
 		return (Node *) newfselect;
 	}
+	if (IsA(node, NullTest))
+	{
+		NullTest   *ntest = (NullTest *) node;
+		NullTest   *newntest;
+		Node	   *arg;
+
+		arg = eval_const_expressions_mutator((Node *) ntest->arg,
+											 context);
+		if (arg && IsA(arg, RowExpr))
+		{
+			RowExpr  *rarg = (RowExpr *) arg;
+			List	*newargs = NIL;
+			ListCell *l;
+
+			/*
+			 * We break ROW(...) IS [NOT] NULL into separate tests on its
+			 * component fields.  This form is usually more efficient to
+			 * evaluate, as well as being more amenable to optimization.
+			 */
+			foreach(l, rarg->args)
+			{
+				Node *relem = (Node *) lfirst(l);
+
+				/*
+				 * A constant field refutes the whole NullTest if it's of
+				 * the wrong nullness; else we can discard it.
+				 */
+				if (relem && IsA(relem, Const))
+				{
+					Const  *carg = (Const *) relem;
+
+					if (carg->constisnull ?
+						(ntest->nulltesttype == IS_NOT_NULL) :
+						(ntest->nulltesttype == IS_NULL))
+						return makeBoolConst(false, false);
+					continue;
+				}
+				newntest = makeNode(NullTest);
+				newntest->arg = (Expr *) relem;
+				newntest->nulltesttype = ntest->nulltesttype;
+				newargs = lappend(newargs, newntest);
+			}
+			/* If all the inputs were constants, result is TRUE */
+			if (newargs == NIL)
+				return makeBoolConst(true, false);
+			/* If only one nonconst input, it's the result */
+			if (list_length(newargs) == 1)
+				return (Node *) linitial(newargs);
+			/* Else we need an AND node */
+			return (Node *) make_andclause(newargs);
+		}
+		if (arg && IsA(arg, Const))
+		{
+			Const  *carg = (Const *) arg;
+			bool	result;
+
+			switch (ntest->nulltesttype)
+			{
+				case IS_NULL:
+					result = carg->constisnull;
+					break;
+				case IS_NOT_NULL:
+					result = !carg->constisnull;
+					break;
+				default:
+					elog(ERROR, "unrecognized nulltesttype: %d",
+						 (int) ntest->nulltesttype);
+					result = false;	/* keep compiler quiet */
+					break;
+			}
+
+			return makeBoolConst(result, false);
+		}
+
+		newntest = makeNode(NullTest);
+		newntest->arg = (Expr *) arg;
+		newntest->nulltesttype = ntest->nulltesttype;
+		return (Node *) newntest;
+	}
 	if (IsA(node, BooleanTest))
 	{
 		BooleanTest *btest = (BooleanTest *) node;

src/backend/optimizer/util/predtest.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/util/predtest.c,v 1.8 2006/08/05 00:21:14 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/util/predtest.c,v 1.9 2006/09/28 20:51:42 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -21,6 +21,7 @@
 #include "executor/executor.h"
 #include "optimizer/clauses.h"
 #include "optimizer/predtest.h"
+#include "parser/parse_expr.h"
 #include "utils/array.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
@@ -931,14 +932,18 @@ predicate_implied_by_simple_clause(Expr *predicate, Node *clause)
 	{
 		Expr	   *nonnullarg = ((NullTest *) predicate)->arg;
 
-		if (is_opclause(clause) &&
-			list_member(((OpExpr *) clause)->args, nonnullarg) &&
-			op_strict(((OpExpr *) clause)->opno))
-			return true;
-		if (is_funcclause(clause) &&
-			list_member(((FuncExpr *) clause)->args, nonnullarg) &&
-			func_strict(((FuncExpr *) clause)->funcid))
-			return true;
+		/* row IS NOT NULL does not act in the simple way we have in mind */
+		if (!type_is_rowtype(exprType((Node *) nonnullarg)))
+		{
+			if (is_opclause(clause) &&
+				list_member(((OpExpr *) clause)->args, nonnullarg) &&
+				op_strict(((OpExpr *) clause)->opno))
+				return true;
+			if (is_funcclause(clause) &&
+				list_member(((FuncExpr *) clause)->args, nonnullarg) &&
+				func_strict(((FuncExpr *) clause)->funcid))
+				return true;
+		}
 		return false;			/* we can't succeed below... */
 	}
 
@@ -978,14 +983,18 @@ predicate_refuted_by_simple_clause(Expr *predicate, Node *clause)
 	{
 		Expr	   *isnullarg = ((NullTest *) predicate)->arg;
 
-		if (is_opclause(clause) &&
-			list_member(((OpExpr *) clause)->args, isnullarg) &&
-			op_strict(((OpExpr *) clause)->opno))
-			return true;
-		if (is_funcclause(clause) &&
-			list_member(((FuncExpr *) clause)->args, isnullarg) &&
-			func_strict(((FuncExpr *) clause)->funcid))
-			return true;
+		/* row IS NULL does not act in the simple way we have in mind */
+		if (!type_is_rowtype(exprType((Node *) isnullarg)))
+		{
+			if (is_opclause(clause) &&
+				list_member(((OpExpr *) clause)->args, isnullarg) &&
+				op_strict(((OpExpr *) clause)->opno))
+				return true;
+			if (is_funcclause(clause) &&
+				list_member(((FuncExpr *) clause)->args, isnullarg) &&
+				func_strict(((FuncExpr *) clause)->funcid))
+				return true;
+		}
 		return false;			/* we can't succeed below... */
 	}