Code review for function default parameters patch. Fix numerous problems as

per recent discussions.  In passing this also fixes a couple of bugs in
the previous variadic-parameters patch.

11 files changed, 515 insertions, 298 deletions

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index e665e9594d..892ca66591 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.114 2008/12/15 18:09:40 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.115 2008/12/18 18:20:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -562,7 +562,8 @@ TypeIsVisible(Oid typid)
  *		retrieve a list of the possible matches.
  *
  * If nargs is -1, we return all functions matching the given name,
- * regardless of argument count. (expand_variadic must be false in this case.)
+ * regardless of argument count.  (expand_variadic and expand_defaults must be
+ * false in this case.)
  *
  * If expand_variadic is true, then variadic functions having the same number
  * or fewer arguments will be retrieved, with the variadic argument and any
@@ -571,23 +572,43 @@ TypeIsVisible(Oid typid)
  * If expand_variadic is false, variadic arguments are not treated specially,
  * and the returned nvargs will always be zero.
  *
- * If expand_variadic is true, functions with argument default values
- * will also be retrieved.  If expand_variadic is false, default
- * values will not be taken into account and functions that do not
- * have exactly nargs arguments in total will not be considered.
+ * If expand_defaults is true, functions that could match after insertion of
+ * default argument values will also be retrieved.  In this case the returned
+ * structs could have nargs > passed-in nargs, and ndargs is set to the number
+ * of additional args (which can be retrieved from the function's
+ * proargdefaults entry).
+ *
+ * It is not possible for nvargs and ndargs to both be nonzero in the same
+ * list entry, since default insertion allows matches to functions with more
+ * than nargs arguments while the variadic transformation requires the same
+ * number or less.
  *
  * We search a single namespace if the function name is qualified, else
- * all namespaces in the search path.  The return list will never contain
- * multiple entries with identical argument lists --- in the multiple-
- * namespace case, we arrange for entries in earlier namespaces to mask
- * identical entries in later namespaces.  We also arrange for non-variadic
- * functions to mask variadic ones if the expanded argument list is the same.
+ * all namespaces in the search path.  In the multiple-namespace case,
+ * we arrange for entries in earlier namespaces to mask identical entries in
+ * later namespaces.
+ *
+ * When expanding variadics, we arrange for non-variadic functions to mask
+ * variadic ones if the expanded argument list is the same.  It is still
+ * possible for there to be conflicts between different variadic functions,
+ * however.
+ *
+ * It is guaranteed that the return list will never contain multiple entries
+ * with identical argument lists.  When expand_defaults is true, the entries
+ * could have more than nargs positions, but we still guarantee that they are
+ * distinct in the first nargs positions.  However, if either expand_variadic
+ * or expand_defaults is true, there might be multiple candidate functions
+ * that expand to identical argument lists.  Rather than throw error here,
+ * we report such situations by setting oid = 0 in the ambiguous entries.
+ * The caller might end up discarding such an entry anyway, but if it selects
+ * such an entry it should react as though the call were ambiguous.
  */
 FuncCandidateList
-FuncnameGetCandidates(List *names, int nargs, bool expand_variadic)
+FuncnameGetCandidates(List *names, int nargs,
+					  bool expand_variadic, bool expand_defaults)
 {
 	FuncCandidateList resultList = NULL;
-	bool		any_variadic = false;
+	bool		any_special = false;
 	char	   *schemaname;
 	char	   *funcname;
 	Oid			namespaceId;
@@ -595,7 +616,7 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic)
 	int			i;
 
 	/* check for caller error */
-	Assert(nargs >= 0 || !expand_variadic);
+	Assert(nargs >= 0 || !(expand_variadic | expand_defaults));
 
 	/* deconstruct the name list */
 	DeconstructQualifiedName(names, &schemaname, &funcname);
@@ -625,42 +646,11 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic)
 		int			effective_nargs;
 		int			pathpos = 0;
 		bool		variadic;
+		bool		use_defaults;
 		Oid			va_elem_type;
-		List	   *defaults = NIL;
 		FuncCandidateList newResult;
 
 		/*
-		 * Check if function has some parameter defaults if some
-		 * parameters are missing.
-		 */
-		if (pronargs > nargs && expand_variadic)
-		{
-			bool		isnull;
-			Datum		proargdefaults;
-			char	   *str;
-
-			/* skip when not enough default expressions */
-			if (nargs + procform->pronargdefaults < pronargs)
-				continue;
-
-			proargdefaults = SysCacheGetAttr(PROCOID, proctup,
-											 Anum_pg_proc_proargdefaults, &isnull);
-			Assert(!isnull);
-			str = TextDatumGetCString(proargdefaults);
-			defaults = (List *) stringToNode(str);
-
-			Assert(IsA(defaults, List));
-
-			/*
-			 * If we don't have to use all default parameters, we skip
-			 * some cells from the left.
-			 */
-			defaults = list_copy_tail(defaults, procform->pronargdefaults - pronargs + nargs);
-
-			pfree(str);
-		}
-
-		/*
 		 * Check if function is variadic, and get variadic element type if so.
 		 * If expand_variadic is false, we should just ignore variadic-ness.
 		 */
@@ -668,6 +658,7 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic)
 		{
 			va_elem_type = procform->provariadic;
 			variadic = OidIsValid(va_elem_type);
+			any_special |= variadic;
 		}
 		else
 		{
@@ -675,16 +666,24 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic)
 			variadic = false;
 		}
 
-		Assert(!variadic || !defaults);
+		/*
+		 * Check if function can match by using parameter defaults.
+		 */
+		if (pronargs > nargs && expand_defaults)
+		{
+			/* Ignore if not enough default expressions */
+			if (nargs + procform->pronargdefaults < pronargs)
+				continue;
+			use_defaults = true;
+			any_special = true;
+		}
+		else
+			use_defaults = false;
 
 		/* Ignore if it doesn't match requested argument count */
-		if (nargs >= 0 &&
-			(variadic ? (pronargs > nargs) : (defaults ? (pronargs < nargs) : (pronargs != nargs))))
+		if (nargs >= 0 && pronargs != nargs && !variadic && !use_defaults)
 			continue;
 
-		Assert(!variadic || (pronargs <= nargs));
-		Assert(!defaults || (pronargs > nargs));
-
 		if (OidIsValid(namespaceId))
 		{
 			/* Consider only procs in specified namespace */
@@ -723,7 +722,6 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic)
 		newResult->pathpos = pathpos;
 		newResult->oid = HeapTupleGetOid(proctup);
 		newResult->nargs = effective_nargs;
-		newResult->argdefaults = defaults;
 		memcpy(newResult->args, procform->proargtypes.values,
 			   pronargs * sizeof(Oid));
 		if (variadic)
@@ -737,128 +735,140 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic)
 		}
 		else
 			newResult->nvargs = 0;
-
-		any_variadic = variadic || defaults;
+		newResult->ndargs = use_defaults ? pronargs - nargs : 0;
 
 		/*
 		 * Does it have the same arguments as something we already accepted?
-		 * If so, decide which one to keep.  We can skip this check for the
-		 * single-namespace case if no variadic match has been made, since
-		 * then the unique index on pg_proc guarantees all the matches have
-		 * different argument lists.
+		 * If so, decide what to do to avoid returning duplicate argument
+		 * lists.  We can skip this check for the single-namespace case if no
+		 * special (variadic or defaults) match has been made, since then the
+		 * unique index on pg_proc guarantees all the matches have different
+		 * argument lists.
 		 */
-		if (any_variadic || !OidIsValid(namespaceId))
+		if (resultList != NULL &&
+			(any_special || !OidIsValid(namespaceId)))
 		{
-			if (defaults)
-				effective_nargs = nargs;
-
 			/*
 			 * If we have an ordered list from SearchSysCacheList (the normal
 			 * case), then any conflicting proc must immediately adjoin this
 			 * one in the list, so we only need to look at the newest result
 			 * item.  If we have an unordered list, we have to scan the whole
 			 * result list.  Also, if either the current candidate or any
-			 * previous candidate is a variadic match, we can't assume that
+			 * previous candidate is a special match, we can't assume that
 			 * conflicts are adjacent.
+			 *
+			 * We ignore defaulted arguments in deciding what is a match.
 			 */
-			if (resultList)
+			FuncCandidateList prevResult;
+
+			if (catlist->ordered && !any_special)
 			{
-				FuncCandidateList prevResult;
+				/* ndargs must be 0 if !any_special */
+				if (effective_nargs == resultList->nargs &&
+					memcmp(newResult->args,
+						   resultList->args,
+						   effective_nargs * sizeof(Oid)) == 0)
+					prevResult = resultList;
+				else
+					prevResult = NULL;
+			}
+			else
+			{
+				int		cmp_nargs = newResult->nargs - newResult->ndargs;
 
-				if (catlist->ordered && !any_variadic)
+				for (prevResult = resultList;
+					 prevResult;
+					 prevResult = prevResult->next)
 				{
-					if (effective_nargs == resultList->nargs &&
+					if (cmp_nargs == prevResult->nargs - prevResult->ndargs &&
 						memcmp(newResult->args,
-							   resultList->args,
-							   effective_nargs * sizeof(Oid)) == 0)
-						prevResult = resultList;
-					else
-						prevResult = NULL;
+							   prevResult->args,
+							   cmp_nargs * sizeof(Oid)) == 0)
+						break;
 				}
-				else
+			}
+
+			if (prevResult)
+			{
+				/*
+				 * We have a match with a previous result.  Decide which one
+				 * to keep, or mark it ambiguous if we can't decide.  The
+				 * logic here is preference > 0 means prefer the old result,
+				 * preference < 0 means prefer the new, preference = 0 means
+				 * ambiguous.
+				 */
+				int		preference;
+
+				if (pathpos != prevResult->pathpos)
 				{
-					for (prevResult = resultList;
-						 prevResult;
-						 prevResult = prevResult->next)
-					{
-						if (!defaults)
-						{
-							if (effective_nargs == prevResult->nargs &&
-								memcmp(newResult->args,
-									    prevResult->args,
-									    effective_nargs * sizeof(Oid)) == 0)
-								break;
-						}
-						else
-						{
-							if (memcmp(newResult->args,
-									    prevResult->args,
-									    effective_nargs * sizeof(Oid)) == 0)
-							break;
-						}
-					}
+					/*
+					 * Prefer the one that's earlier in the search path.
+					 */
+					preference = pathpos - prevResult->pathpos;
 				}
-				if (prevResult)
+				else if (variadic && prevResult->nvargs == 0)
 				{
 					/*
-					 * We have a match with a previous result.  Prefer the
-					 * one that's earlier in the search path.
+					 * With variadic functions we could have, for example,
+					 * both foo(numeric) and foo(variadic numeric[]) in
+					 * the same namespace; if so we prefer the
+					 * non-variadic match on efficiency grounds.
 					 */
-					if (pathpos > prevResult->pathpos)
-					{
-						pfree(newResult);
-						continue;		/* keep previous result */
-					}
-					else if (pathpos == prevResult->pathpos)
+					preference = 1;
+				}
+				else if (!variadic && prevResult->nvargs > 0)
+				{
+					preference = -1;
+				}
+				else
+				{
+					/*----------
+					 * We can't decide.  This can happen with, for example,
+					 * both foo(numeric, variadic numeric[]) and
+					 * foo(variadic numeric[]) in the same namespace, or
+					 * both foo(int) and foo (int, int default something)
+					 * in the same namespace.
+					 *----------
+					 */
+					preference = 0;
+				}
+
+				if (preference > 0)
+				{
+					/* keep previous result */
+					pfree(newResult);
+					continue;
+				}
+				else if (preference < 0)
+				{
+					/* remove previous result from the list */
+					if (prevResult == resultList)
+						resultList = prevResult->next;
+					else
 					{
-						/*
-						 * With variadic functions we could have, for example,
-						 * both foo(numeric) and foo(variadic numeric[]) in
-						 * the same namespace; if so we prefer the
-						 * non-variadic match on efficiency grounds.  It's
-						 * also possible to have conflicting variadic
-						 * functions, such as foo(numeric, variadic numeric[])
-						 * and foo(variadic numeric[]).  If you're silly
-						 * enough to do that, we throw an error.  (XXX It'd be
-						 * better to detect such conflicts when the functions
-						 * are created.)
-						 */
-						if (variadic)
-						{
-							if (prevResult->nvargs > 0)
-								ereport(ERROR,
-										(errcode(ERRCODE_AMBIGUOUS_FUNCTION),
-										 errmsg("variadic function %s conflicts with another",
-												func_signature_string(names, pronargs,
-																	  procform->proargtypes.values))));
-							/* else, previous result wasn't variadic */
-							pfree(newResult);
-							continue;	/* keep previous result */
-						}
+						FuncCandidateList prevPrevResult;
 
-						if (defaults)
+						for (prevPrevResult = resultList;
+							 prevPrevResult;
+							 prevPrevResult = prevPrevResult->next)
 						{
-							if (prevResult->argdefaults != NIL)
-								ereport(ERROR,
-										(errcode(ERRCODE_AMBIGUOUS_FUNCTION),
-										 errmsg("functions with parameter defaults %s and %s are ambiguous",
-												func_signature_string(names, pronargs, procform->proargtypes.values),
-												func_signature_string(names, prevResult->nargs, prevResult->args))));
-							/* else, previous result didn't have defaults */
-							pfree(newResult);
-							continue;	/* keep previous result */
+							if (prevResult == prevPrevResult->next)
+							{
+								prevPrevResult->next = prevResult->next;
+								break;
+							}
 						}
-
-						/* non-variadic can replace a previous variadic */
-						Assert(prevResult->nvargs > 0);
+						Assert(prevPrevResult);		/* assert we found it */
 					}
-					/* replace previous result */
-					prevResult->pathpos = pathpos;
-					prevResult->oid = newResult->oid;
-					prevResult->nvargs = newResult->nvargs;
-					prevResult->argdefaults = newResult->argdefaults;
+					pfree(prevResult);
+					/* fall through to add newResult to list */
+				}
+				else
+				{
+					/* mark old result as ambiguous, discard new */
+					prevResult->oid = InvalidOid;
 					pfree(newResult);
-					continue;	/* args are same, of course */
+					continue;
 				}
 			}
 		}
@@ -922,7 +932,7 @@ FunctionIsVisible(Oid funcid)
 		visible = false;
 
 		clist = FuncnameGetCandidates(list_make1(makeString(proname)),
-									  nargs, false);
+									  nargs, false, false);
 
 		for (; clist; clist = clist->next)
 		{
@@ -1191,6 +1201,7 @@ OpernameGetCandidates(List *names, char oprkind)
 		newResult->oid = HeapTupleGetOid(opertup);
 		newResult->nargs = 2;
 		newResult->nvargs = 0;
+		newResult->ndargs = 0;
 		newResult->args[0] = operform->oprleft;
 		newResult->args[1] = operform->oprright;
 		newResult->next = resultList;
diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index 7cd0d513ed..414366bc54 100644
--- a/src/backend/catalog/pg_aggregate.c
+++ b/src/backend/catalog/pg_aggregate.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/pg_aggregate.c,v 1.98 2008/12/04 17:51:26 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/pg_aggregate.c,v 1.99 2008/12/18 18:20:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -227,10 +227,10 @@ AggregateCreate(const char *aggName,
 							  PointerGetDatum(NULL),	/* allParamTypes */
 							  PointerGetDatum(NULL),	/* parameterModes */
 							  PointerGetDatum(NULL),	/* parameterNames */
+							  NIL,						/* parameterDefaults */
 							  PointerGetDatum(NULL),	/* proconfig */
 							  1,	/* procost */
-							  0,		/* prorows */
-							  NULL);	/* parameterDefaults */
+							  0);		/* prorows */
 
 	/*
 	 * Okay to create the pg_aggregate entry.
@@ -320,7 +320,7 @@ lookup_agg_function(List *fnName,
 	 * function's return value.  it also returns the true argument types to
 	 * the function.
 	 */
-	fdresult = func_get_detail(fnName, NIL, nargs, input_types, false,
+	fdresult = func_get_detail(fnName, NIL, nargs, input_types, false, false,
 							   &fnOid, rettype, &retset, &nvargs,
 							   &true_oid_array, NULL);
 
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 1b414e3e5a..f9339fd2ce 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.155 2008/12/04 17:51:26 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.156 2008/12/18 18:20:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -27,6 +27,7 @@
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
+#include "nodes/nodeFuncs.h"
 #include "parser/parse_type.h"
 #include "tcop/pquery.h"
 #include "tcop/tcopprot.h"
@@ -73,10 +74,10 @@ ProcedureCreate(const char *procedureName,
 				Datum allParameterTypes,
 				Datum parameterModes,
 				Datum parameterNames,
+				List *parameterDefaults,
 				Datum proconfig,
 				float4 procost,
-				float4 prorows,
-				List *parameterDefaults)
+				float4 prorows)
 {
 	Oid			retval;
 	int			parameterCount;
@@ -311,11 +312,8 @@ ProcedureCreate(const char *procedureName,
 		values[Anum_pg_proc_proargnames - 1] = parameterNames;
 	else
 		nulls[Anum_pg_proc_proargnames - 1] = true;
-	if (parameterDefaults != PointerGetDatum(NULL))
-	{
-		Assert(list_length(parameterDefaults) > 0);
+	if (parameterDefaults != NIL)
 		values[Anum_pg_proc_proargdefaults - 1] = CStringGetTextDatum(nodeToString(parameterDefaults));
-	}
 	else
 		nulls[Anum_pg_proc_proargdefaults - 1] = true;
 	values[Anum_pg_proc_prosrc - 1] = CStringGetTextDatum(prosrc);
@@ -389,6 +387,57 @@ ProcedureCreate(const char *procedureName,
 						 errhint("Use DROP FUNCTION first.")));
 		}
 
+		/*
+		 * If there are existing defaults, check compatibility: redefinition
+		 * must not remove any defaults nor change their types.  (Removing
+		 * a default might cause a function to fail to satisfy an existing
+		 * call.  Changing type would only be possible if the associated
+		 * parameter is polymorphic, and in such cases a change of default
+		 * type might alter the resolved output type of existing calls.)
+		 */
+		if (oldproc->pronargdefaults != 0)
+		{
+			Datum		proargdefaults;
+			bool		isnull;
+			List	   *oldDefaults;
+			ListCell   *oldlc;
+			ListCell   *newlc;
+
+			if (list_length(parameterDefaults) < oldproc->pronargdefaults)
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+						 errmsg("cannot remove parameter defaults from existing function"),
+						 errhint("Use DROP FUNCTION first.")));
+
+			proargdefaults = SysCacheGetAttr(PROCNAMEARGSNSP, oldtup,
+											 Anum_pg_proc_proargdefaults,
+											 &isnull);
+			Assert(!isnull);
+			oldDefaults = (List *) stringToNode(TextDatumGetCString(proargdefaults));
+			Assert(IsA(oldDefaults, List));
+			Assert(list_length(oldDefaults) == oldproc->pronargdefaults);
+
+			/* new list can have more defaults than old, advance over 'em */
+			newlc = list_head(parameterDefaults);
+			for (i = list_length(parameterDefaults) - oldproc->pronargdefaults;
+				 i > 0;
+				 i--)
+				newlc = lnext(newlc);
+
+			foreach(oldlc, oldDefaults)
+			{
+				Node   *oldDef = (Node *) lfirst(oldlc);
+				Node   *newDef = (Node *) lfirst(newlc);
+
+				if (exprType(oldDef) != exprType(newDef))
+					ereport(ERROR,
+							(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+							 errmsg("cannot change data type of existing parameter default value"),
+							 errhint("Use DROP FUNCTION first.")));
+				newlc = lnext(newlc);
+			}
+		}
+
 		/* Can't change aggregate status, either */
 		if (oldproc->proisagg != isAgg)
 		{
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 1509da7312..0a3de53e1e 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/functioncmds.c,v 1.102 2008/12/04 17:51:26 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/functioncmds.c,v 1.103 2008/12/18 18:20:33 tgl Exp $
  *
  * DESCRIPTION
  *	  These routines take the parse tree and pick out the
@@ -48,11 +48,11 @@
 #include "commands/defrem.h"
 #include "commands/proclang.h"
 #include "miscadmin.h"
+#include "optimizer/var.h"
 #include "parser/parse_coerce.h"
 #include "parser/parse_expr.h"
 #include "parser/parse_func.h"
 #include "parser/parse_type.h"
-#include "parser/parse_utilcmd.h"
 #include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
@@ -162,13 +162,13 @@ compute_return_type(TypeName *returnType, Oid languageOid,
  */
 static void
 examine_parameter_list(List *parameters, Oid languageOid,
+					   const char *queryString,
 					   oidvector **parameterTypes,
 					   ArrayType **allParameterTypes,
 					   ArrayType **parameterModes,
 					   ArrayType **parameterNames,
 					   List **parameterDefaults,
-					   Oid *requiredResultType,
-					   const char *queryString)
+					   Oid *requiredResultType)
 {
 	int			parameterCount = list_length(parameters);
 	Oid		   *inTypes;
@@ -179,9 +179,9 @@ examine_parameter_list(List *parameters, Oid languageOid,
 	int			outCount = 0;
 	int			varCount = 0;
 	bool		have_names = false;
+	bool		have_defaults = false;
 	ListCell   *x;
 	int			i;
-	bool		have_defaults = false;
 	ParseState *pstate;
 
 	*requiredResultType = InvalidOid;	/* default result */
@@ -192,6 +192,7 @@ examine_parameter_list(List *parameters, Oid languageOid,
 	paramNames = (Datum *) palloc0(parameterCount * sizeof(Datum));
 	*parameterDefaults = NIL;
 
+	/* may need a pstate for parse analysis of default exprs */
 	pstate = make_parsestate(NULL);
 	pstate->p_sourcetext = queryString;
 
@@ -201,6 +202,7 @@ examine_parameter_list(List *parameters, Oid languageOid,
 	{
 		FunctionParameter *fp = (FunctionParameter *) lfirst(x);
 		TypeName   *t = fp->argType;
+		bool		isinput = false;
 		Oid			toid;
 		Type		typtup;
 
@@ -247,6 +249,7 @@ examine_parameter_list(List *parameters, Oid languageOid,
 						(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
 						 errmsg("VARIADIC parameter must be the last input parameter")));
 			inTypes[inCount++] = toid;
+			isinput = true;
 		}
 
 		/* handle output parameters */
@@ -288,24 +291,46 @@ examine_parameter_list(List *parameters, Oid languageOid,
 
 		if (fp->defexpr)
 		{
-			if (fp->mode != FUNC_PARAM_IN && fp->mode != FUNC_PARAM_INOUT)
+			Node   *def;
+
+			if (!isinput)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
-						 errmsg("only IN and INOUT parameters can have default values")));
+						 errmsg("only input parameters can have default values")));
+
+			def = transformExpr(pstate, fp->defexpr);
+			def = coerce_to_specific_type(pstate, def, toid, "DEFAULT");
+
+			/*
+			 * Make sure no variables are referred to.
+			 */
+			if (list_length(pstate->p_rtable) != 0 ||
+				contain_var_clause(def))
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
+						 errmsg("cannot use table references in parameter default value")));
+
+			/*
+			 * No subplans or aggregates, either...
+			 */
+			if (pstate->p_hasSubLinks)
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("cannot use subquery in parameter default value")));
+			if (pstate->p_hasAggs)
+				ereport(ERROR,
+						(errcode(ERRCODE_GROUPING_ERROR),
+						 errmsg("cannot use aggregate function in parameter default value")));
 
-			*parameterDefaults = lappend(*parameterDefaults,
-										 coerce_to_specific_type(NULL,
-																 transformExpr(pstate, fp->defexpr),
-																 toid,
-																 "DEFAULT"));
+			*parameterDefaults = lappend(*parameterDefaults, def);
 			have_defaults = true;
 		}
 		else
 		{
-			if (have_defaults)
+			if (isinput && have_defaults)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
-						 errmsg("parameter without default value specified after parameter with default value")));
+						 errmsg("input parameters after one with a default value must also have defaults")));
 		}
 
 		i++;
@@ -704,6 +729,7 @@ CreateFunction(CreateFunctionStmt *stmt, const char *queryString)
 	ArrayType  *allParameterTypes;
 	ArrayType  *parameterModes;
 	ArrayType  *parameterNames;
+	List	   *parameterDefaults;
 	Oid			requiredResultType;
 	bool		isStrict,
 				security;
@@ -714,7 +740,6 @@ CreateFunction(CreateFunctionStmt *stmt, const char *queryString)
 	HeapTuple	languageTuple;
 	Form_pg_language languageStruct;
 	List	   *as_clause;
-	List		*defaults = NULL;
 
 	/* Convert list of names to a name and namespace */
 	namespaceId = QualifiedNameGetCreationNamespace(stmt->funcname,
@@ -783,14 +808,13 @@ CreateFunction(CreateFunctionStmt *stmt, const char *queryString)
 	 * Convert remaining parameters of CREATE to form wanted by
 	 * ProcedureCreate.
 	 */
-	examine_parameter_list(stmt->parameters, languageOid,
+	examine_parameter_list(stmt->parameters, languageOid, queryString,
 						   &parameterTypes,
 						   &allParameterTypes,
 						   &parameterModes,
 						   &parameterNames,
-						   &defaults,
-						   &requiredResultType,
-						   queryString);
+						   &parameterDefaults,
+						   &requiredResultType);
 
 	if (stmt->returnType)
 	{
@@ -871,10 +895,10 @@ CreateFunction(CreateFunctionStmt *stmt, const char *queryString)
 					PointerGetDatum(allParameterTypes),
 					PointerGetDatum(parameterModes),
 					PointerGetDatum(parameterNames),
+					parameterDefaults,
 					PointerGetDatum(proconfig),
 					procost,
-					prorows,
-					defaults);
+					prorows);
 }
 
 
diff --git a/src/backend/commands/proclang.c b/src/backend/commands/proclang.c
index 2166a1de5f..3188156c4d 100644
--- a/src/backend/commands/proclang.c
+++ b/src/backend/commands/proclang.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/proclang.c,v 1.81 2008/12/04 17:51:26 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/proclang.c,v 1.82 2008/12/18 18:20:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -147,10 +147,10 @@ CreateProceduralLanguage(CreatePLangStmt *stmt)
 										 PointerGetDatum(NULL),
 										 PointerGetDatum(NULL),
 										 PointerGetDatum(NULL),
+										 NIL,
 										 PointerGetDatum(NULL),
 										 1,
-										 0,
-										 NULL);
+										 0);
 		}
 
 		/*
@@ -181,10 +181,10 @@ CreateProceduralLanguage(CreatePLangStmt *stmt)
 										 PointerGetDatum(NULL),
 										 PointerGetDatum(NULL),
 										 PointerGetDatum(NULL),
+										 NIL,
 										 PointerGetDatum(NULL),
 										 1,
-										 0,
-										 NULL);
+										 0);
 			}
 		}
 		else
@@ -548,9 +548,9 @@ AlterLanguageOwner(const char *name, Oid newOwnerId)
 				 errmsg("language \"%s\" does not exist", name)));
 
 	AlterLanguageOwner_internal(tup, rel, newOwnerId);
-	
+
 	ReleaseSysCache(tup);
-	
+
 	heap_close(rel, RowExclusiveLock);
 
 }
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 467ff39a31..7f91309032 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.246 2008/10/22 20:17:51 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.247 2008/12/18 18:20:33 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -513,27 +513,18 @@ preprocess_expression(PlannerInfo *root, Node *expr, int kind)
 	/*
 	 * Simplify constant expressions.
 	 *
+	 * Note: one essential effect here is to insert the current actual values
+	 * of any default arguments for functions.  To ensure that happens, we
+	 * *must* process all expressions here.  Previous PG versions sometimes
+	 * skipped const-simplification if it didn't seem worth the trouble, but
+	 * we can't do that anymore.
+	 *
 	 * Note: this also flattens nested AND and OR expressions into N-argument
 	 * form.  All processing of a qual expression after this point must be
 	 * careful to maintain AND/OR flatness --- that is, do not generate a tree
 	 * with AND directly under AND, nor OR directly under OR.
-	 *
-	 * Because this is a relatively expensive process, we skip it when the
-	 * query is trivial, such as "SELECT 2+2;" or "INSERT ... VALUES()". The
-	 * expression will only be evaluated once anyway, so no point in
-	 * pre-simplifying; we can't execute it any faster than the executor can,
-	 * and we will waste cycles copying the tree.  Notice however that we
-	 * still must do it for quals (to get AND/OR flatness); and if we are in a
-	 * subquery we should not assume it will be done only once.
-	 *
-	 * For VALUES lists we never do this at all, again on the grounds that we
-	 * should optimize for one-time evaluation.
 	 */
-	if (kind != EXPRKIND_VALUES &&
-		(root->parse->jointree->fromlist != NIL ||
-		 kind == EXPRKIND_QUAL ||
-		 root->query_level > 1))
-		expr = eval_const_expressions(root, expr);
+	expr = eval_const_expressions(root, expr);
 
 	/*
 	 * If it's a qual or havingQual, canonicalize it.
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index c826ecb2ad..3c74831f4d 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.270 2008/10/21 20:42:53 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.271 2008/12/18 18:20:34 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -36,6 +36,7 @@
 #include "optimizer/var.h"
 #include "parser/analyze.h"
 #include "parser/parse_coerce.h"
+#include "parser/parse_func.h"
 #include "rewrite/rewriteManip.h"
 #include "tcop/tcopprot.h"
 #include "utils/acl.h"
@@ -91,9 +92,11 @@ static List *simplify_and_arguments(List *args,
 					   bool *haveNull, bool *forceFalse);
 static Expr *simplify_boolean_equality(List *args);
 static Expr *simplify_function(Oid funcid,
-				  Oid result_type, int32 result_typmod, List *args,
+				  Oid result_type, int32 result_typmod, List **args,
 				  bool allow_inline,
 				  eval_const_expressions_context *context);
+static List *add_function_defaults(List *args, Oid result_type,
+								   HeapTuple func_tuple);
 static Expr *evaluate_function(Oid funcid,
 				  Oid result_type, int32 result_typmod, List *args,
 				  HeapTuple func_tuple,
@@ -2025,7 +2028,7 @@ eval_const_expressions_mutator(Node *node,
 		 */
 		simple = simplify_function(expr->funcid,
 								   expr->funcresulttype, exprTypmod(node),
-								   args,
+								   &args,
 								   true, context);
 		if (simple)				/* successfully simplified it */
 			return (Node *) simple;
@@ -2072,7 +2075,7 @@ eval_const_expressions_mutator(Node *node,
 		 */
 		simple = simplify_function(expr->opfuncid,
 								   expr->opresulttype, -1,
-								   args,
+								   &args,
 								   true, context);
 		if (simple)				/* successfully simplified it */
 			return (Node *) simple;
@@ -2163,7 +2166,7 @@ eval_const_expressions_mutator(Node *node,
 			 */
 			simple = simplify_function(expr->opfuncid,
 									   expr->opresulttype, -1,
-									   args,
+									   &args,
 									   false, context);
 			if (simple)			/* successfully simplified it */
 			{
@@ -2329,6 +2332,7 @@ eval_const_expressions_mutator(Node *node,
 	{
 		CoerceViaIO *expr = (CoerceViaIO *) node;
 		Expr	   *arg;
+		List	   *args;
 		Oid			outfunc;
 		bool		outtypisvarlena;
 		Oid			infunc;
@@ -2341,6 +2345,7 @@ eval_const_expressions_mutator(Node *node,
 		 */
 		arg = (Expr *) eval_const_expressions_mutator((Node *) expr->arg,
 													  context);
+		args = list_make1(arg);
 
 		/*
 		 * CoerceViaIO represents calling the source type's output function
@@ -2353,7 +2358,7 @@ eval_const_expressions_mutator(Node *node,
 
 		simple = simplify_function(outfunc,
 								   CSTRINGOID, -1,
-								   list_make1(arg),
+								   &args,
 								   true, context);
 		if (simple)				/* successfully simplified output fn */
 		{
@@ -2361,8 +2366,6 @@ eval_const_expressions_mutator(Node *node,
 			 * Input functions may want 1 to 3 arguments.  We always supply
 			 * all three, trusting that nothing downstream will complain.
 			 */
-			List	   *args;
-
 			args = list_make3(simple,
 							  makeConst(OIDOID, -1, sizeof(Oid),
 										ObjectIdGetDatum(intypioparam),
@@ -2373,7 +2376,7 @@ eval_const_expressions_mutator(Node *node,
 
 			simple = simplify_function(infunc,
 									   expr->resulttype, -1,
-									   args,
+									   &args,
 									   true, context);
 			if (simple)			/* successfully simplified input fn */
 				return (Node *) simple;
@@ -3126,10 +3129,16 @@ simplify_boolean_equality(List *args)
  *
  * Returns a simplified expression if successful, or NULL if cannot
  * simplify the function call.
+ *
+ * This function is also responsible for adding any default argument
+ * expressions onto the function argument list; which is a bit grotty,
+ * but it avoids an extra fetch of the function's pg_proc tuple.  For this
+ * reason, the args list is pass-by-reference, and it may get modified
+ * even if simplification fails.
  */
 static Expr *
 simplify_function(Oid funcid, Oid result_type, int32 result_typmod,
-				  List *args,
+				  List **args,
 				  bool allow_inline,
 				  eval_const_expressions_context *context)
 {
@@ -3150,11 +3159,15 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod,
 	if (!HeapTupleIsValid(func_tuple))
 		elog(ERROR, "cache lookup failed for function %u", funcid);
 
-	newexpr = evaluate_function(funcid, result_type, result_typmod, args,
+	/* While we have the tuple, check if we need to add defaults */
+	if (((Form_pg_proc) GETSTRUCT(func_tuple))->pronargs > list_length(*args))
+		*args = add_function_defaults(*args, result_type, func_tuple);
+
+	newexpr = evaluate_function(funcid, result_type, result_typmod, *args,
 								func_tuple, context);
 
 	if (!newexpr && allow_inline)
-		newexpr = inline_function(funcid, result_type, args,
+		newexpr = inline_function(funcid, result_type, *args,
 								  func_tuple, context);
 
 	ReleaseSysCache(func_tuple);
@@ -3163,6 +3176,77 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod,
 }
 
 /*
+ * add_function_defaults: add missing function arguments from its defaults
+ *
+ * It is possible for some of the defaulted arguments to be polymorphic;
+ * therefore we can't assume that the default expressions have the correct
+ * data types already.  We have to re-resolve polymorphics and do coercion
+ * just like the parser did.
+ */
+static List *
+add_function_defaults(List *args, Oid result_type, HeapTuple func_tuple)
+{
+	Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
+	Datum		proargdefaults;
+	bool		isnull;
+	char	   *str;
+	List	   *defaults;
+	int			ndelete;
+	int			nargs;
+	Oid			actual_arg_types[FUNC_MAX_ARGS];
+	Oid			declared_arg_types[FUNC_MAX_ARGS];
+	Oid			rettype;
+	ListCell   *lc;
+
+	/* The error cases here shouldn't happen, but check anyway */
+	proargdefaults = SysCacheGetAttr(PROCOID, func_tuple,
+									 Anum_pg_proc_proargdefaults,
+									 &isnull);
+	if (isnull)
+		elog(ERROR, "not enough default arguments");
+	str = TextDatumGetCString(proargdefaults);
+	defaults = (List *) stringToNode(str);
+	Assert(IsA(defaults, List));
+	pfree(str);
+	/* Delete any unused defaults from the list */
+	ndelete = list_length(args) + list_length(defaults) - funcform->pronargs;
+	if (ndelete < 0)
+		elog(ERROR, "not enough default arguments");
+	while (ndelete-- > 0)
+		defaults = list_delete_first(defaults);
+	/* And form the combined argument list */
+	args = list_concat(args, defaults);
+	Assert(list_length(args) == funcform->pronargs);
+
+	/*
+	 * The rest of this should be a no-op if there are no polymorphic
+	 * arguments, but we do it anyway to be sure.
+	 */
+	if (list_length(args) > FUNC_MAX_ARGS)
+		elog(ERROR, "too many function arguments");
+	nargs = 0;
+	foreach(lc, args)
+	{
+		actual_arg_types[nargs++] = exprType((Node *) lfirst(lc));
+	}
+	memcpy(declared_arg_types, funcform->proargtypes.values,
+		   funcform->pronargs * sizeof(Oid));
+	rettype = enforce_generic_type_consistency(actual_arg_types,
+											   declared_arg_types,
+											   nargs,
+											   funcform->prorettype,
+											   false);
+	/* let's just check we got the same answer as the parser did ... */
+	if (rettype != result_type)
+		elog(ERROR, "function's resolved result type changed during planning");
+
+	/* perform any necessary typecasting of arguments */
+	make_fn_arguments(NULL, args, actual_arg_types, declared_arg_types);
+
+	return args;
+}
+
+/*
  * evaluate_function: try to pre-evaluate a function call
  *
  * We can do this if the function is strict and has any constant-null inputs
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e53333d30c..337af63327 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.644 2008/12/17 09:15:02 heikki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.645 2008/12/18 18:20:34 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -3465,7 +3465,7 @@ opt_restart_seqs:
  *
  *	COMMENT ON [ [ DATABASE | DOMAIN | INDEX | SEQUENCE | TABLE | TYPE | VIEW |
  *				   CONVERSION | LANGUAGE | OPERATOR CLASS | LARGE OBJECT |
- *				   CAST | COLUMN | SCHEMA | TABLESPACE | ROLE | 
+ *				   CAST | COLUMN | SCHEMA | TABLESPACE | ROLE |
  *				   TEXT SEARCH PARSER | TEXT SEARCH DICTIONARY |
  *				   TEXT SEARCH TEMPLATE |
  *				   TEXT SEARCH CONFIGURATION ] <objname> |
@@ -4230,15 +4230,15 @@ func_args_list:
  */
 func_args_with_defaults:
 		'(' func_args_with_defaults_list ')'		{ $$ = $2; }
-			| '(' ')'								{ $$ = NIL; }
+		| '(' ')'									{ $$ = NIL; }
 		;
 
 func_args_with_defaults_list:
-		func_arg_with_default						{ $$ = list_make1( $1); }
-			| func_args_with_defaults_list ',' func_arg_with_default	{ $$ = lappend($1, $3); }
+		func_arg_with_default						{ $$ = list_make1($1); }
+		| func_args_with_defaults_list ',' func_arg_with_default
+													{ $$ = lappend($1, $3); }
 		;
 
-
 /*
  * The style with arg_class first is SQL99 standard, but Oracle puts
  * param_name first; accept both since it's likely people will try both
@@ -4345,7 +4345,7 @@ func_type:	Typename								{ $$ = $1; }
 
 func_arg_with_default:
 		func_arg
-			    { 
+			    {
 				    $$ = $1;
 			    }
 		| func_arg DEFAULT a_expr
@@ -4459,6 +4459,7 @@ table_func_column:	param_name func_type
 					n->name = $1;
 					n->argType = $2;
 					n->mode = FUNC_PARAM_TABLE;
+					n->defexpr = NULL;
 					$$ = n;
 				}
 		;
@@ -5755,7 +5756,7 @@ AlterTSConfigurationStmt:
 					n->replace = false;
 					$$ = (Node*)n;
 				}
-			| ALTER TEXT_P SEARCH CONFIGURATION any_name ALTER MAPPING REPLACE any_name WITH any_name 
+			| ALTER TEXT_P SEARCH CONFIGURATION any_name ALTER MAPPING REPLACE any_name WITH any_name
 				{
 					AlterTSConfigurationStmt *n = makeNode(AlterTSConfigurationStmt);
 					n->cfgname = $5;
@@ -5765,7 +5766,7 @@ AlterTSConfigurationStmt:
 					n->replace = true;
 					$$ = (Node*)n;
 				}
-			| ALTER TEXT_P SEARCH CONFIGURATION any_name ALTER MAPPING FOR name_list REPLACE any_name WITH any_name 
+			| ALTER TEXT_P SEARCH CONFIGURATION any_name ALTER MAPPING FOR name_list REPLACE any_name WITH any_name
 				{
 					AlterTSConfigurationStmt *n = makeNode(AlterTSConfigurationStmt);
 					n->cfgname = $5;
@@ -5775,7 +5776,7 @@ AlterTSConfigurationStmt:
 					n->replace = true;
 					$$ = (Node*)n;
 				}
-			| ALTER TEXT_P SEARCH CONFIGURATION any_name DROP MAPPING FOR name_list 
+			| ALTER TEXT_P SEARCH CONFIGURATION any_name DROP MAPPING FOR name_list
 				{
 					AlterTSConfigurationStmt *n = makeNode(AlterTSConfigurationStmt);
 					n->cfgname = $5;
@@ -5783,7 +5784,7 @@ AlterTSConfigurationStmt:
 					n->missing_ok = false;
 					$$ = (Node*)n;
 				}
-			| ALTER TEXT_P SEARCH CONFIGURATION any_name DROP MAPPING IF_P EXISTS FOR name_list 
+			| ALTER TEXT_P SEARCH CONFIGURATION any_name DROP MAPPING IF_P EXISTS FOR name_list
 				{
 					AlterTSConfigurationStmt *n = makeNode(AlterTSConfigurationStmt);
 					n->cfgname = $5;
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 1171d367c6..d0b74ff5d9 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.208 2008/12/04 17:51:26 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.209 2008/12/18 18:20:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -71,13 +71,14 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 	ListCell   *nextl;
 	Node	   *first_arg = NULL;
 	int			nargs;
+	int			nargsplusdefs;
 	Oid			actual_arg_types[FUNC_MAX_ARGS];
 	Oid		   *declared_arg_types;
+	List	   *argdefaults;
 	Node	   *retval;
 	bool		retset;
 	int			nvargs;
 	FuncDetailCode fdresult;
-	List	   *argdefaults;
 
 	/*
 	 * Most of the rest of the parser just assumes that functions do not have
@@ -157,13 +158,13 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 	 * disambiguation for polymorphic functions, handles inheritance, and
 	 * returns the funcid and type and set or singleton status of the
 	 * function's return value.  It also returns the true argument types to
-	 * the function.  (In the case of a variadic function call, the reported
+	 * the function.  In the case of a variadic function call, the reported
 	 * "true" types aren't really what is in pg_proc: the variadic argument is
 	 * replaced by a suitable number of copies of its element type.  We'll fix
-	 * it up below.)
+	 * it up below.  We may also have to deal with default arguments.
 	 */
 	fdresult = func_get_detail(funcname, fargs, nargs, actual_arg_types,
-							   !func_variadic,
+							   !func_variadic, true,
 							   &funcid, &rettype, &retset, &nvargs,
 							   &declared_arg_types, &argdefaults);
 	if (fdresult == FUNCDETAIL_COERCION)
@@ -235,20 +236,28 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 					 parser_errposition(pstate, location)));
 	}
 
-	/* add stored expressions as called values for arguments with defaults */
-	if (argdefaults)
+	/*
+	 * If there are default arguments, we have to include their types in
+	 * actual_arg_types for the purpose of checking generic type consistency.
+	 * However, we do NOT put them into the generated parse node, because
+	 * their actual values might change before the query gets run.  The
+	 * planner has to insert the up-to-date values at plan time.
+	 */
+	nargsplusdefs = nargs;
+	foreach(l, argdefaults)
 	{
-		ListCell  *lc;
+		Node	*expr = (Node *) lfirst(l);
 
-		foreach(lc, argdefaults)
-		{
-			Node	*expr = (Node *) lfirst(lc);
-    
-			fargs = lappend(fargs, expr);
-			actual_arg_types[nargs++] = exprType(expr);
-		}
-	}
+		/* probably shouldn't happen ... */
+		if (nargsplusdefs >= FUNC_MAX_ARGS)
+			ereport(ERROR,
+					(errcode(ERRCODE_TOO_MANY_ARGUMENTS),
+					 errmsg("cannot pass more than %d arguments to a function",
+							FUNC_MAX_ARGS),
+					 parser_errposition(pstate, location)));
 
+		actual_arg_types[nargsplusdefs++] = exprType(expr);
+	}
 
 	/*
 	 * enforce consistency with polymorphic argument and return types,
@@ -257,7 +266,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 	 */
 	rettype = enforce_generic_type_consistency(actual_arg_types,
 											   declared_arg_types,
-											   nargs,
+											   nargsplusdefs,
 											   rettype,
 											   false);
 
@@ -741,18 +750,20 @@ func_get_detail(List *funcname,
 				int nargs,
 				Oid *argtypes,
 				bool expand_variadic,
+				bool expand_defaults,
 				Oid *funcid,	/* return value */
 				Oid *rettype,	/* return value */
 				bool *retset,	/* return value */
 				int *nvargs,	/* return value */
 				Oid **true_typeids,		/* return value */
-				List **argdefaults)	/* return value */
+				List **argdefaults)		/* optional return value */
 {
 	FuncCandidateList raw_candidates;
 	FuncCandidateList best_candidate;
 
 	/* Get list of possible candidates from namespace search */
-	raw_candidates = FuncnameGetCandidates(funcname, nargs, expand_variadic);
+	raw_candidates = FuncnameGetCandidates(funcname, nargs,
+										   expand_variadic, expand_defaults);
 
 	/*
 	 * Quickly check if there is an exact match to the input datatypes (there
@@ -884,11 +895,17 @@ func_get_detail(List *funcname,
 		Form_pg_proc pform;
 		FuncDetailCode result;
 
+		/*
+		 * If expanding variadics or defaults, the "best candidate" might
+		 * represent multiple equivalently good functions; treat this case
+		 * as ambiguous.
+		 */
+		if (!OidIsValid(best_candidate->oid))
+			return FUNCDETAIL_MULTIPLE;
+
 		*funcid = best_candidate->oid;
 		*nvargs = best_candidate->nvargs;
 		*true_typeids = best_candidate->args;
-		if (argdefaults)
-			*argdefaults = best_candidate->argdefaults;
 
 		ftup = SearchSysCache(PROCOID,
 							  ObjectIdGetDatum(best_candidate->oid),
@@ -899,6 +916,38 @@ func_get_detail(List *funcname,
 		pform = (Form_pg_proc) GETSTRUCT(ftup);
 		*rettype = pform->prorettype;
 		*retset = pform->proretset;
+		/* fetch default args if caller wants 'em */
+		if (argdefaults)
+		{
+			if (best_candidate->ndargs > 0)
+			{
+				Datum		proargdefaults;
+				bool		isnull;
+				char	   *str;
+				List	   *defaults;
+				int			ndelete;
+
+				/* shouldn't happen, FuncnameGetCandidates messed up */
+				if (best_candidate->ndargs > pform->pronargdefaults)
+					elog(ERROR, "not enough default arguments");
+
+				proargdefaults = SysCacheGetAttr(PROCOID, ftup,
+												 Anum_pg_proc_proargdefaults,
+												 &isnull);
+				Assert(!isnull);
+				str = TextDatumGetCString(proargdefaults);
+				defaults = (List *) stringToNode(str);
+				Assert(IsA(defaults, List));
+				pfree(str);
+				/* Delete any unused defaults from the returned list */
+				ndelete = list_length(defaults) - best_candidate->ndargs;
+				while (ndelete-- > 0)
+					defaults = list_delete_first(defaults);
+				*argdefaults = defaults;
+			}
+			else
+				*argdefaults = NIL;
+		}
 		result = pform->proisagg ? FUNCDETAIL_AGGREGATE : FUNCDETAIL_NORMAL;
 		ReleaseSysCache(ftup);
 		return result;
@@ -1243,7 +1292,7 @@ LookupFuncName(List *funcname, int nargs, const Oid *argtypes, bool noError)
 {
 	FuncCandidateList clist;
 
-	clist = FuncnameGetCandidates(funcname, nargs, false);
+	clist = FuncnameGetCandidates(funcname, nargs, false, false);
 
 	while (clist)
 	{
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index d50dc23d77..b690c19273 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -13,7 +13,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/regproc.c,v 1.108 2008/07/16 01:30:22 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/regproc.c,v 1.109 2008/12/18 18:20:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -131,7 +131,7 @@ regprocin(PG_FUNCTION_ARGS)
 	 * pg_proc entries in the current search path.
 	 */
 	names = stringToQualifiedNameList(pro_name_or_oid);
-	clist = FuncnameGetCandidates(names, -1, false);
+	clist = FuncnameGetCandidates(names, -1, false, false);
 
 	if (clist == NULL)
 		ereport(ERROR,
@@ -190,7 +190,7 @@ regprocout(PG_FUNCTION_ARGS)
 			 * qualify it.
 			 */
 			clist = FuncnameGetCandidates(list_make1(makeString(proname)),
-										  -1, false);
+										  -1, false, false);
 			if (clist != NULL && clist->next == NULL &&
 				clist->oid == proid)
 				nspname = NULL;
@@ -277,7 +277,7 @@ regprocedurein(PG_FUNCTION_ARGS)
 	 */
 	parseNameAndArgTypes(pro_name_or_oid, false, &names, &nargs, argtypes);
 
-	clist = FuncnameGetCandidates(names, nargs, false);
+	clist = FuncnameGetCandidates(names, nargs, false, false);
 
 	for (; clist; clist = clist->next)
 	{
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index afa19b384e..5a6540b88e 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.288 2008/12/04 17:51:27 petere Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.289 2008/12/18 18:20:34 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -141,8 +141,7 @@ static char *pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
 static char *pg_get_expr_worker(text *expr, Oid relid, char *relname,
 				   int prettyFlags);
 static int print_function_arguments(StringInfo buf, HeapTuple proctup,
-						 bool print_table_args,
-						 bool full);
+						 bool print_table_args, bool print_defaults);
 static void print_function_rettype(StringInfo buf, HeapTuple proctup);
 static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
 			 int prettyFlags);
@@ -1610,7 +1609,7 @@ pg_get_function_arguments(PG_FUNCTION_ARGS)
  * pg_get_function_identity_arguments
  *		Get a formatted list of arguments for a function.
  *		This is everything that would go between the parentheses in
- *		ALTER FUNCTION, etc. skip names and defaults/
+ *		ALTER FUNCTION, etc.  In particular, don't print defaults.
  */
 Datum
 pg_get_function_identity_arguments(PG_FUNCTION_ARGS)
@@ -1634,8 +1633,6 @@ pg_get_function_identity_arguments(PG_FUNCTION_ARGS)
 	PG_RETURN_TEXT_P(string_to_text(buf.data));
 }
 
-
-
 /*
  * pg_get_function_result
  *		Get a nicely-formatted version of the result type of a function.
@@ -1680,7 +1677,7 @@ print_function_rettype(StringInfo buf, HeapTuple proctup)
 	{
 		/* It might be a table function; try to print the arguments */
 		appendStringInfoString(&rbuf, "TABLE(");
-		ntabargs = print_function_arguments(&rbuf, proctup, true, true);
+		ntabargs = print_function_arguments(&rbuf, proctup, true, false);
 		if (ntabargs > 0)
 			appendStringInfoString(&rbuf, ")");
 		else
@@ -1702,100 +1699,112 @@ print_function_rettype(StringInfo buf, HeapTuple proctup)
  * Common code for pg_get_function_arguments and pg_get_function_result:
  * append the desired subset of arguments to buf.  We print only TABLE
  * arguments when print_table_args is true, and all the others when it's false.
+ * We print argument defaults only if print_defaults is true.
  * Function return value is the number of arguments printed.
- * When full is false, then don't print argument names and argument defaults.
  */
 static int
 print_function_arguments(StringInfo buf, HeapTuple proctup,
-						 bool print_table_args,
-						 bool full)
+						 bool print_table_args, bool print_defaults)
 {
+	Form_pg_proc proc = (Form_pg_proc) GETSTRUCT(proctup);
 	int			numargs;
 	Oid		   *argtypes;
 	char	  **argnames;
 	char	   *argmodes;
 	int			argsprinted;
+	int			inputargno;
+	int			nlackdefaults;
+	ListCell   *nextargdefault = NULL;
 	int			i;
-	Datum		proargdefaults;
-	List	   *argdefaults;
-	int			nargdefaults;
-	bool		isnull;
-	List	   *dcontext = NIL;
 
 	numargs = get_func_arg_info(proctup,
 								&argtypes, &argnames, &argmodes);
 
-	proargdefaults = SysCacheGetAttr(PROCOID, proctup,
-									 Anum_pg_proc_proargdefaults, &isnull);
-	if (!isnull)
-	{
-		char	*str;
-
-		str = TextDatumGetCString(proargdefaults);
-		argdefaults = (List *) stringToNode(str);
-		Assert(IsA(argdefaults, List));
-		nargdefaults = list_length(argdefaults);
-	    
-		/* we will need deparse context */
-		//dcontext = deparse_context_for("", InvalidOid);
-		dcontext = NULL;
-		pfree(str);
-	}
-	else
+	nlackdefaults = numargs;
+	if (print_defaults && proc->pronargdefaults > 0)
 	{
-		argdefaults = NIL;
-		nargdefaults = 0;
+		Datum		proargdefaults;
+		bool		isnull;
+
+		proargdefaults = SysCacheGetAttr(PROCOID, proctup,
+										 Anum_pg_proc_proargdefaults,
+										 &isnull);
+		if (!isnull)
+		{
+			char   *str;
+			List   *argdefaults;
+
+			str = TextDatumGetCString(proargdefaults);
+			argdefaults = (List *) stringToNode(str);
+			Assert(IsA(argdefaults, List));
+			pfree(str);
+			nextargdefault = list_head(argdefaults);
+			/* nlackdefaults counts only *input* arguments lacking defaults */
+			nlackdefaults = proc->pronargs - list_length(argdefaults);
+		}
 	}
 
 	argsprinted = 0;
+	inputargno = 0;
 	for (i = 0; i < numargs; i++)
 	{
 		Oid		argtype = argtypes[i];
 		char   *argname = argnames ? argnames[i] : NULL;
 		char	argmode = argmodes ? argmodes[i] : PROARGMODE_IN;
 		const char *modename;
-
-		if (print_table_args != (argmode == PROARGMODE_TABLE))
-			continue;
+		bool	isinput;
 
 		switch (argmode)
 		{
 			case PROARGMODE_IN:
 				modename = "";
+				isinput = true;
 				break;
 			case PROARGMODE_INOUT:
 				modename = "INOUT ";
+				isinput = true;
 				break;
 			case PROARGMODE_OUT:
 				modename = "OUT ";
+				isinput = false;
 				break;
 			case PROARGMODE_VARIADIC:
 				modename = "VARIADIC ";
+				isinput = true;
 				break;
 			case PROARGMODE_TABLE:
 				modename = "";
+				isinput = false;
 				break;
 			default:
 				elog(ERROR, "invalid parameter mode '%c'", argmode);
 				modename = NULL; /* keep compiler quiet */
+				isinput = false;
 				break;
 		}
+		if (isinput)
+			inputargno++;		/* this is a 1-based counter */
+
+		if (print_table_args != (argmode == PROARGMODE_TABLE))
+			continue;
+
 		if (argsprinted)
 			appendStringInfoString(buf, ", ");
 		appendStringInfoString(buf, modename);
-		if (argname && argname[0] && full)
+		if (argname && argname[0])
 			appendStringInfo(buf, "%s ", argname);
 		appendStringInfoString(buf, format_type_be(argtype));
-
-		/* search given default expression, expect less numargs */
-		if (nargdefaults > 0 && i >= (numargs - nargdefaults) && full)
+		if (print_defaults && isinput && inputargno > nlackdefaults)
 		{
 			Node	*expr;
 
-			expr = (Node *) list_nth(argdefaults, i - (numargs - nargdefaults));
-			appendStringInfo(buf, " DEFAULT %s",  deparse_expression(expr, dcontext, false, false));
-		}
+			Assert(nextargdefault != NULL);
+			expr = (Node *) lfirst(nextargdefault);
+			nextargdefault = lnext(nextargdefault);
 
+			appendStringInfo(buf, " DEFAULT %s",
+							 deparse_expression(expr, NIL, false, false));
+		}
 		argsprinted++;
 	}
 
@@ -6062,7 +6071,6 @@ generate_function_name(Oid funcid, int nargs, Oid *argtypes,
 		elog(ERROR, "cache lookup failed for function %u", funcid);
 	procform = (Form_pg_proc) GETSTRUCT(proctup);
 	proname = NameStr(procform->proname);
-	Assert(nargs >= procform->pronargs);
 
 	/*
 	 * The idea here is to schema-qualify only if the parser would fail to
@@ -6070,7 +6078,7 @@ generate_function_name(Oid funcid, int nargs, Oid *argtypes,
 	 * specified argtypes.
 	 */
 	p_result = func_get_detail(list_make1(makeString(proname)),
-							   NIL, nargs, argtypes, false,
+							   NIL, nargs, argtypes, false, true,
 							   &p_funcid, &p_rettype,
 							   &p_retset, &p_nvargs, &p_true_typeids, NULL);
 	if ((p_result == FUNCDETAIL_NORMAL || p_result == FUNCDETAIL_AGGREGATE) &&