From a43f4307f78bd80e88cdfb5f36ad59819c4b552e Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 10 Jan 2004 18:13:53 +0000
Subject: [PATCH] Improve has_nullable_targetlist() to allow strict functions
 of simple variables, not just simple variables.  This was foreseen in the
 original coding of this routine, but not implemented until now.  Responds to
 performance gripe from Laurent Perez.

---
 src/backend/optimizer/prep/prepjointree.c | 24 +++++++++-----
 src/backend/optimizer/util/clauses.c      | 40 ++++++++++++-----------
 2 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 90485171d9..033aaba494 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -16,7 +16,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.15 2004/01/10 00:30:21 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.16 2004/01/10 18:13:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -439,11 +439,13 @@ is_simple_subquery(Query *subquery)
 /*
  * has_nullable_targetlist
  *	  Check a subquery in the range table to see if all the non-junk
- *	  targetlist items are simple variables (and, hence, will correctly
- *	  go to NULL when examined above the point of an outer join).
+ *	  targetlist items are simple variables or strict functions of simple
+ *	  variables (and, hence, will correctly go to NULL when examined above
+ *	  the point of an outer join).
  *
- * A possible future extension is to accept strict functions of simple
- * variables, eg, "x + 1".
+ * NOTE: it would be correct (and useful) to ignore output columns that aren't
+ * actually referenced by the enclosing query ... but we do not have that
+ * information available at this point.
  */
 static bool
 has_nullable_targetlist(Query *subquery)
@@ -458,11 +460,15 @@ has_nullable_targetlist(Query *subquery)
 		if (tle->resdom->resjunk)
 			continue;
 
-		/* Okay if tlist item is a simple Var */
-		if (tle->expr && IsA(tle->expr, Var))
-			continue;
+		/* Must contain a Var of current level */
+		if (!contain_vars_of_level((Node *) tle->expr, 0))
+			return false;
 
-		return false;
+		/* Must not contain any non-strict constructs */
+		if (contain_nonstrict_functions((Node *) tle->expr))
+			return false;
+
+		/* This one's OK, keep scanning */
 	}
 	return true;
 }
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index b2b2d7d02b..6ce171e2f1 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.160 2004/01/05 18:04:39 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.161 2004/01/10 18:13:53 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -697,10 +697,10 @@ contain_volatile_functions_walker(Node *node, void *context)
  * Returns true if any nonstrict construct is found --- ie, anything that
  * could produce non-NULL output with a NULL input.
  *
- * XXX we do not examine sub-selects to see if they contain uses of
- * nonstrict functions. It's not real clear if that is correct or not...
- * for the current usage it does not matter, since inline_function()
- * rejects cases with sublinks.
+ * The idea here is that the caller has verified that the expression contains
+ * one or more Var or Param nodes (as appropriate for the caller's need), and
+ * now wishes to prove that the expression result will be NULL if any of these
+ * inputs is NULL.  If we return false, then the proof succeeded.
  */
 bool
 contain_nonstrict_functions(Node *clause)
@@ -713,6 +713,11 @@ contain_nonstrict_functions_walker(Node *node, void *context)
 {
 	if (node == NULL)
 		return false;
+	if (IsA(node, Aggref))
+	{
+		/* an aggregate could return non-null with null input */
+		return true;
+	}
 	if (IsA(node, FuncExpr))
 	{
 		FuncExpr   *expr = (FuncExpr *) node;
@@ -745,16 +750,25 @@ contain_nonstrict_functions_walker(Node *node, void *context)
 
 		switch (expr->boolop)
 		{
-			case OR_EXPR:
 			case AND_EXPR:
-				/* OR, AND are inherently non-strict */
+			case OR_EXPR:
+				/* AND, OR are inherently non-strict */
 				return true;
 			default:
 				break;
 		}
 	}
+	if (IsA(node, SubLink))
+	{
+		/* In some cases a sublink might be strict, but in general not */
+		return true;
+	}
+	if (IsA(node, SubPlan))
+		return true;
 	if (IsA(node, CaseExpr))
 		return true;
+	if (IsA(node, CaseWhen))
+		return true;
 	/* NB: ArrayExpr might someday be nonstrict */
 	if (IsA(node, CoalesceExpr))
 		return true;
@@ -764,18 +778,6 @@ contain_nonstrict_functions_walker(Node *node, void *context)
 		return true;
 	if (IsA(node, BooleanTest))
 		return true;
-	if (IsA(node, SubLink))
-	{
-		SubLink    *sublink = (SubLink *) node;
-		List	   *opid;
-
-		foreach(opid, sublink->operOids)
-		{
-			if (!op_strict(lfirsto(opid)))
-				return true;
-		}
-		/* else fall through to check args */
-	}
 	return expression_tree_walker(node, contain_nonstrict_functions_walker,
 								  context);
 }