Fix a bug introduced when set-returning SQL functions were made inline-able:

we have to cope with the possibility that the declared result rowtype contains
dropped columns.  This fails in 8.4, as per bug #5240.

While at it, be more paranoid about inserting binary coercions when inlining.
The pre-8.4 code did not really need to worry about that because it could not
inline at all in any case where an added coercion could change the behavior
of the function's statement.  However, when inlining a SRF we allow sorting,
grouping, and set-ops such as UNION.  In these cases, modifying one of the
targetlist entries that the sort/group/setop depends on could conceivably
change the behavior of the function's statement --- so don't inline when
such a case applies.

1 files changed, 17 insertions, 5 deletions

diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index dcfc731a17a..a5c343298e6 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.279 2009/10/08 02:39:21 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.280 2009/12/14 02:15:52 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -3673,6 +3673,7 @@ inline_function(Oid funcid, Oid result_type, List *args,
 	char	   *src;
 	Datum		tmp;
 	bool		isNull;
+	bool		modifyTargetList;
 	MemoryContext oldcxt;
 	MemoryContext mycxt;
 	ErrorContextCallback sqlerrcontext;
@@ -3792,7 +3793,7 @@ inline_function(Oid funcid, Oid result_type, List *args,
 	 * needed; that's probably not important, but let's be careful.
 	 */
 	if (check_sql_fn_retval(funcid, result_type, list_make1(querytree),
-							true, NULL))
+							&modifyTargetList, NULL))
 		goto fail;				/* reject whole-tuple-result cases */
 
 	/* Now we can grab the tlist expression */
@@ -3800,6 +3801,8 @@ inline_function(Oid funcid, Oid result_type, List *args,
 
 	/* Assert that check_sql_fn_retval did the right thing */
 	Assert(exprType(newexpr) == result_type);
+	/* It couldn't have made any dangerous tlist changes, either */
+	Assert(!modifyTargetList);
 
 	/*
 	 * Additional validity checks on the expression.  It mustn't return a set,
@@ -4088,6 +4091,7 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	char	   *src;
 	Datum		tmp;
 	bool		isNull;
+	bool		modifyTargetList;
 	MemoryContext oldcxt;
 	MemoryContext mycxt;
 	ErrorContextCallback sqlerrcontext;
@@ -4253,8 +4257,8 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	 * Make sure the function (still) returns what it's declared to.  This
 	 * will raise an error if wrong, but that's okay since the function would
 	 * fail at runtime anyway.	Note that check_sql_fn_retval will also insert
-	 * RelabelType(s) if needed to make the tlist expression(s) match the
-	 * declared type of the function.
+	 * RelabelType(s) and/or NULL columns if needed to make the tlist
+	 * expression(s) match the declared type of the function.
 	 *
 	 * If the function returns a composite type, don't inline unless the check
 	 * shows it's returning a whole tuple result; otherwise what it's
@@ -4262,12 +4266,20 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
 	 */
 	if (!check_sql_fn_retval(func_oid, fexpr->funcresulttype,
 							 querytree_list,
-							 true, NULL) &&
+							 &modifyTargetList, NULL) &&
 		(get_typtype(fexpr->funcresulttype) == TYPTYPE_COMPOSITE ||
 		 fexpr->funcresulttype == RECORDOID))
 		goto fail;				/* reject not-whole-tuple-result cases */
 
 	/*
+	 * If we had to modify the tlist to make it match, and the statement is
+	 * one in which changing the tlist contents could change semantics, we
+	 * have to punt and not inline.
+	 */
+	if (modifyTargetList)
+		goto fail;
+
+	/*
 	 * If it returns RECORD, we have to check against the column type list
 	 * provided in the RTE; check_sql_fn_retval can't do that.  (If no match,
 	 * we just fail to inline, rather than complaining; see notes for