Fix problems with SQL functions returning rowtypes that have dropped

columns.  The returned tuple needs to have appropriate NULL columns
inserted so that it actually matches the declared rowtype.  It seemed
convenient to use a JunkFilter for this, so I made some cleanups and
simplifications in the JunkFilter code to allow it to support this
additional functionality.  (That in turn exposed a latent bug in
nodeAppend.c, which is that it was returning a tuple slot whose
descriptor didn't match its data.)  Also, move check_sql_fn_retval
out of pg_proc.c and into functions.c, where it seems to more naturally
belong.

1 files changed, 296 insertions, 14 deletions

diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 1db5a4339ff..d1683a91cb4 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.89 2004/09/13 20:06:46 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.90 2004/10/07 18:38:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -18,10 +18,11 @@
 #include "catalog/pg_proc.h"
 #include "catalog/pg_type.h"
 #include "commands/trigger.h"
-#include "executor/execdefs.h"
 #include "executor/executor.h"
 #include "executor/functions.h"
-#include "tcop/pquery.h"
+#include "parser/parse_coerce.h"
+#include "parser/parse_expr.h"
+#include "parser/parse_type.h"
 #include "tcop/tcopprot.h"
 #include "tcop/utility.h"
 #include "utils/builtins.h"
@@ -69,6 +70,8 @@ typedef struct
 
 	ParamListInfo paramLI;		/* Param list representing current args */
 
+	JunkFilter *junkFilter;		/* used only if returnsTuple */
+
 	/* head of linked list of execution_state records */
 	execution_state *func_state;
 } SQLFunctionCache;
@@ -268,11 +271,16 @@ init_sql_fcache(FmgrInfo *finfo)
 	 * result, or just regurgitating a rowtype expression result. In the
 	 * latter case we clear returnsTuple because we need not act different
 	 * from the scalar result case.
+	 *
+	 * In the returnsTuple case, check_sql_fn_retval will also construct
+	 * a JunkFilter we can use to coerce the returned rowtype to the desired
+	 * form.
 	 */
 	if (haspolyarg || fcache->returnsTuple)
 		fcache->returnsTuple = check_sql_fn_retval(rettype,
 												   get_typtype(rettype),
-												   queryTree_list);
+												   queryTree_list,
+												   &fcache->junkFilter);
 
 	/* Finally, plan the queries */
 	fcache->func_state = init_execution_state(queryTree_list,
@@ -477,24 +485,40 @@ postquel_execute(execution_state *es,
 	/*
 	 * Set up to return the function value.
 	 */
-	tup = slot->val;
-	tupDesc = slot->ttc_tupleDescriptor;
-
 	if (fcache->returnsTuple)
 	{
 		/*
-		 * We are returning the whole tuple, so copy it into current
-		 * execution context and make sure it is a valid Datum.
+		 * We are returning the whole tuple, so filter it and apply the
+		 * proper labeling to make it a valid Datum.  There are several
+		 * reasons why we do this:
 		 *
-		 * XXX do we need to remove junk attrs from the result tuple?
-		 * Probably OK to leave them, as long as they are at the end.
+		 * 1. To copy the tuple out of the child execution context and
+		 * into our own context.
+		 *
+		 * 2. To remove any junk attributes present in the raw subselect
+		 * result.  (This is probably not absolutely necessary, but it
+		 * seems like good policy.)
+		 *
+		 * 3. To insert dummy null columns if the declared result type
+		 * has any attisdropped columns.
 		 */
+		HeapTuple	newtup;
 		HeapTupleHeader dtup;
+		uint32		t_len;
 		Oid			dtuptype;
 		int32		dtuptypmod;
 
-		dtup = (HeapTupleHeader) palloc(tup->t_len);
-		memcpy((char *) dtup, (char *) tup->t_data, tup->t_len);
+		newtup = ExecRemoveJunk(fcache->junkFilter, slot);
+
+		/*
+		 * Compress out the HeapTuple header data.  We assume that
+		 * heap_formtuple made the tuple with header and body in one
+		 * palloc'd chunk.  We want to return a pointer to the chunk
+		 * start so that it will work if someone tries to free it.
+		 */
+		t_len = newtup->t_len;
+		dtup = (HeapTupleHeader) newtup;
+		memmove((char *) dtup, (char *) newtup->t_data, t_len);
 
 		/*
 		 * Use the declared return type if it's not RECORD; else take
@@ -510,6 +534,7 @@ postquel_execute(execution_state *es,
 		else
 		{
 			/* function is declared to return RECORD */
+			tupDesc = fcache->junkFilter->jf_cleanTupType;
 			if (tupDesc->tdtypeid == RECORDOID &&
 				tupDesc->tdtypmod < 0)
 				assign_record_type_typmod(tupDesc);
@@ -517,7 +542,7 @@ postquel_execute(execution_state *es,
 			dtuptypmod = tupDesc->tdtypmod;
 		}
 
-		HeapTupleHeaderSetDatumLength(dtup, tup->t_len);
+		HeapTupleHeaderSetDatumLength(dtup, t_len);
 		HeapTupleHeaderSetTypeId(dtup, dtuptype);
 		HeapTupleHeaderSetTypMod(dtup, dtuptypmod);
 
@@ -531,6 +556,9 @@ postquel_execute(execution_state *es,
 		 * column of the SELECT result, and then copy into current
 		 * execution context if needed.
 		 */
+		tup = slot->val;
+		tupDesc = slot->ttc_tupleDescriptor;
+
 		value = heap_getattr(tup, 1, tupDesc, &(fcinfo->isnull));
 
 		if (!fcinfo->isnull)
@@ -808,3 +836,257 @@ ShutdownSQLFunction(Datum arg)
 	/* execUtils will deregister the callback... */
 	fcache->shutdown_reg = false;
 }
+
+
+/*
+ * check_sql_fn_retval() -- check return value of a list of sql parse trees.
+ *
+ * The return value of a sql function is the value returned by
+ * the final query in the function.  We do some ad-hoc type checking here
+ * to be sure that the user is returning the type he claims.
+ *
+ * This is normally applied during function definition, but in the case
+ * of a function with polymorphic arguments, we instead apply it during
+ * function execution startup.	The rettype is then the actual resolved
+ * output type of the function, rather than the declared type.	(Therefore,
+ * we should never see ANYARRAY or ANYELEMENT as rettype.)
+ *
+ * The return value is true if the function returns the entire tuple result
+ * of its final SELECT, and false otherwise.  Note that because we allow
+ * "SELECT rowtype_expression", this may be false even when the declared
+ * function return type is a rowtype.
+ *
+ * If junkFilter isn't NULL, then *junkFilter is set to a JunkFilter defined
+ * to convert the function's tuple result to the correct output tuple type.
+ * Whenever the result value is false (ie, the function isn't returning a
+ * tuple result), *junkFilter is set to NULL.
+ */
+bool
+check_sql_fn_retval(Oid rettype, char fn_typtype, List *queryTreeList,
+					JunkFilter **junkFilter)
+{
+	Query	   *parse;
+	int			cmd;
+	List	   *tlist;
+	ListCell   *tlistitem;
+	int			tlistlen;
+	Oid			typerelid;
+	Oid			restype;
+	Relation	reln;
+	int			relnatts;		/* physical number of columns in rel */
+	int			rellogcols;		/* # of nondeleted columns in rel */
+	int			colindex;		/* physical column index */
+
+	if (junkFilter)
+		*junkFilter = NULL;		/* default result */
+
+	/* guard against empty function body; OK only if void return type */
+	if (queryTreeList == NIL)
+	{
+		if (rettype != VOIDOID)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+					 errmsg("return type mismatch in function declared to return %s",
+							format_type_be(rettype)),
+			 errdetail("Function's final statement must be a SELECT.")));
+		return false;
+	}
+
+	/* find the final query */
+	parse = (Query *) lfirst(list_tail(queryTreeList));
+
+	cmd = parse->commandType;
+	tlist = parse->targetList;
+
+	/*
+	 * The last query must be a SELECT if and only if return type isn't
+	 * VOID.
+	 */
+	if (rettype == VOIDOID)
+	{
+		if (cmd == CMD_SELECT)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+					 errmsg("return type mismatch in function declared to return %s",
+							format_type_be(rettype)),
+					 errdetail("Function's final statement must not be a SELECT.")));
+		return false;
+	}
+
+	/* by here, the function is declared to return some type */
+	if (cmd != CMD_SELECT)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+		 errmsg("return type mismatch in function declared to return %s",
+				format_type_be(rettype)),
+			 errdetail("Function's final statement must be a SELECT.")));
+
+	/*
+	 * Count the non-junk entries in the result targetlist.
+	 */
+	tlistlen = ExecCleanTargetListLength(tlist);
+
+	typerelid = typeidTypeRelid(rettype);
+
+	if (fn_typtype == 'b' || fn_typtype == 'd')
+	{
+		/* Shouldn't have a typerelid */
+		Assert(typerelid == InvalidOid);
+
+		/*
+		 * For base-type returns, the target list should have exactly one
+		 * entry, and its type should agree with what the user declared.
+		 * (As of Postgres 7.2, we accept binary-compatible types too.)
+		 */
+		if (tlistlen != 1)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+					 errmsg("return type mismatch in function declared to return %s",
+							format_type_be(rettype)),
+			 errdetail("Final SELECT must return exactly one column.")));
+
+		restype = ((TargetEntry *) linitial(tlist))->resdom->restype;
+		if (!IsBinaryCoercible(restype, rettype))
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+					 errmsg("return type mismatch in function declared to return %s",
+							format_type_be(rettype)),
+					 errdetail("Actual return type is %s.",
+							   format_type_be(restype))));
+	}
+	else if (fn_typtype == 'c')
+	{
+		/* Must have a typerelid */
+		Assert(typerelid != InvalidOid);
+
+		/*
+		 * If the target list is of length 1, and the type of the varnode
+		 * in the target list matches the declared return type, this is
+		 * okay. This can happen, for example, where the body of the
+		 * function is 'SELECT func2()', where func2 has the same return
+		 * type as the function that's calling it.
+		 */
+		if (tlistlen == 1)
+		{
+			restype = ((TargetEntry *) linitial(tlist))->resdom->restype;
+			if (IsBinaryCoercible(restype, rettype))
+				return false;	/* NOT returning whole tuple */
+		}
+
+		/*
+		 * Otherwise verify that the targetlist matches the return tuple
+		 * type. This part of the typechecking is a hack. We look up the
+		 * relation that is the declared return type, and scan the
+		 * non-deleted attributes to ensure that they match the datatypes
+		 * of the non-resjunk columns.
+		 */
+		reln = relation_open(typerelid, AccessShareLock);
+		relnatts = reln->rd_rel->relnatts;
+		rellogcols = 0;			/* we'll count nondeleted cols as we go */
+		colindex = 0;
+
+		foreach(tlistitem, tlist)
+		{
+			TargetEntry *tle = (TargetEntry *) lfirst(tlistitem);
+			Form_pg_attribute attr;
+			Oid			tletype;
+			Oid			atttype;
+
+			if (tle->resdom->resjunk)
+				continue;
+
+			do
+			{
+				colindex++;
+				if (colindex > relnatts)
+					ereport(ERROR,
+							(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+							 errmsg("return type mismatch in function declared to return %s",
+									format_type_be(rettype)),
+					errdetail("Final SELECT returns too many columns.")));
+				attr = reln->rd_att->attrs[colindex - 1];
+			} while (attr->attisdropped);
+			rellogcols++;
+
+			tletype = exprType((Node *) tle->expr);
+			atttype = attr->atttypid;
+			if (!IsBinaryCoercible(tletype, atttype))
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+						 errmsg("return type mismatch in function declared to return %s",
+								format_type_be(rettype)),
+						 errdetail("Final SELECT returns %s instead of %s at column %d.",
+								   format_type_be(tletype),
+								   format_type_be(atttype),
+								   rellogcols)));
+		}
+
+		for (;;)
+		{
+			colindex++;
+			if (colindex > relnatts)
+				break;
+			if (!reln->rd_att->attrs[colindex - 1]->attisdropped)
+				rellogcols++;
+		}
+
+		if (tlistlen != rellogcols)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+					 errmsg("return type mismatch in function declared to return %s",
+							format_type_be(rettype)),
+					 errdetail("Final SELECT returns too few columns.")));
+
+		/* Set up junk filter if needed */
+		if (junkFilter)
+			*junkFilter = ExecInitJunkFilterConversion(tlist,
+											CreateTupleDescCopy(reln->rd_att),
+											NULL);
+
+		relation_close(reln, AccessShareLock);
+
+		/* Report that we are returning entire tuple result */
+		return true;
+	}
+	else if (rettype == RECORDOID)
+	{
+		/*
+		 * If the target list is of length 1, and the type of the varnode
+		 * in the target list matches the declared return type, this is
+		 * okay. This can happen, for example, where the body of the
+		 * function is 'SELECT func2()', where func2 has the same return
+		 * type as the function that's calling it.
+		 */
+		if (tlistlen == 1)
+		{
+			restype = ((TargetEntry *) linitial(tlist))->resdom->restype;
+			if (IsBinaryCoercible(restype, rettype))
+				return false;	/* NOT returning whole tuple */
+		}
+
+		/*
+		 * Otherwise assume we are returning the whole tuple.
+		 * Crosschecking against what the caller expects will happen at
+		 * runtime.
+		 */
+		if (junkFilter)
+			*junkFilter = ExecInitJunkFilter(tlist, false, NULL);
+
+		return true;
+	}
+	else if (rettype == ANYARRAYOID || rettype == ANYELEMENTOID)
+	{
+		/* This should already have been caught ... */
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+				 errmsg("cannot determine result data type"),
+				 errdetail("A function returning \"anyarray\" or \"anyelement\" must have at least one argument of either type.")));
+	}
+	else
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+			  errmsg("return type %s is not supported for SQL functions",
+					 format_type_be(rettype))));
+
+	return false;
+}