Hi list,

Per Tom's request, I split out this refactoring from my CacheExpr patch.

Basically I'm just centralizing the eval_const_expressions_mutator()
call on function arguments, from multiple different places to a single
location. Without this, it would be a lot harder to implement argument
caching logic in the CacheExpr patch.

The old call tree goes like:
case T_FuncExpr:
-> eval_const_expressions_mutator(args)
-> simplify_function
   -> reorder_function_arguments
       -> eval_const_expressions_mutator(args)
   -> add_function_defaults
       -> eval_const_expressions_mutator(args)

New call tree:
case T_FuncExpr:
-> simplify_function
   -> simplify_copy_function_arguments
      -> reorder_function_arguments
      -> add_function_defaults
   -> eval_const_expressions_mutator(args)

Assumptions being made:
* The code didn't depend on expanding existing arguments before adding defaults
* operator calls don't need to expand default arguments -- it's
currently impossible to CREATE OPERATOR with a function that has
unspecified arguments

Other changes:
* simplify_function no longer needs a 'bool has_named_args' argument
(it finds out itself).
* I added 'bool mutate_args' in its place, since some callers need to
mutate args beforehand.
* reorder_function_arguments no longer needs to keep track of which
default args were added.

Regards,
Marti
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
new file mode 100644
index cd3da46..9485e95
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
*************** static List *simplify_and_arguments(List
*** 109,124 ****
  static Node *simplify_boolean_equality(Oid opno, List *args);
  static Expr *simplify_function(Expr *oldexpr, Oid funcid,
  				  Oid result_type, int32 result_typmod, Oid result_collid,
! 				  Oid input_collid, List **args,
! 				  bool has_named_args,
! 				  bool allow_inline,
  				  eval_const_expressions_context *context);
  static List *reorder_function_arguments(List *args, Oid result_type,
! 						   HeapTuple func_tuple,
! 						   eval_const_expressions_context *context);
  static List *add_function_defaults(List *args, Oid result_type,
! 					  HeapTuple func_tuple,
! 					  eval_const_expressions_context *context);
  static List *fetch_function_defaults(HeapTuple func_tuple);
  static void recheck_cast_function_args(List *args, Oid result_type,
  						   HeapTuple func_tuple);
--- 109,123 ----
  static Node *simplify_boolean_equality(Oid opno, List *args);
  static Expr *simplify_function(Expr *oldexpr, Oid funcid,
  				  Oid result_type, int32 result_typmod, Oid result_collid,
! 				  Oid input_collid, List **args_p,
! 				  bool mutate_args, bool allow_inline,
  				  eval_const_expressions_context *context);
+ static List *simplify_copy_function_arguments(List *old_args, Oid result_type,
+ 							HeapTuple func_tuple);
  static List *reorder_function_arguments(List *args, Oid result_type,
! 						   HeapTuple func_tuple);
  static List *add_function_defaults(List *args, Oid result_type,
! 					  HeapTuple func_tuple);
  static List *fetch_function_defaults(HeapTuple func_tuple);
  static void recheck_cast_function_args(List *args, Oid result_type,
  						   HeapTuple func_tuple);
*************** eval_const_expressions_mutator(Node *nod
*** 2303,2336 ****
  		case T_FuncExpr:
  			{
  				FuncExpr   *expr = (FuncExpr *) node;
! 				List	   *args;
! 				bool		has_named_args;
  				Expr	   *simple;
  				FuncExpr   *newexpr;
- 				ListCell   *lc;
- 
- 				/*
- 				 * Reduce constants in the FuncExpr's arguments, and check to
- 				 * see if there are any named args.
- 				 */
- 				args = NIL;
- 				has_named_args = false;
- 				foreach(lc, expr->args)
- 				{
- 					Node	   *arg = (Node *) lfirst(lc);
- 
- 					arg = eval_const_expressions_mutator(arg, context);
- 					if (IsA(arg, NamedArgExpr))
- 						has_named_args = true;
- 					args = lappend(args, arg);
- 				}
  
  				/*
  				 * Code for op/func reduction is pretty bulky, so split it out
  				 * as a separate function.	Note: exprTypmod normally returns
  				 * -1 for a FuncExpr, but not when the node is recognizably a
  				 * length coercion; we want to preserve the typmod in the
! 				 * eventual Const if so.
  				 */
  				simple = simplify_function((Expr *) expr,
  										   expr->funcid,
--- 2302,2318 ----
  		case T_FuncExpr:
  			{
  				FuncExpr   *expr = (FuncExpr *) node;
! 				List	   *args = expr->args;
  				Expr	   *simple;
  				FuncExpr   *newexpr;
  
  				/*
  				 * Code for op/func reduction is pretty bulky, so split it out
  				 * as a separate function.	Note: exprTypmod normally returns
  				 * -1 for a FuncExpr, but not when the node is recognizably a
  				 * length coercion; we want to preserve the typmod in the
! 				 * eventual Const if so. This function also mutates the
! 				 * argument list.
  				 */
  				simple = simplify_function((Expr *) expr,
  										   expr->funcid,
*************** eval_const_expressions_mutator(Node *nod
*** 2339,2345 ****
  										   expr->funccollid,
  										   expr->inputcollid,
  										   &args,
! 										   has_named_args,
  										   true,
  										   context);
  				if (simple)		/* successfully simplified it */
--- 2321,2327 ----
  										   expr->funccollid,
  										   expr->inputcollid,
  										   &args,
! 										   true,
  										   true,
  										   context);
  				if (simple)		/* successfully simplified it */
*************** eval_const_expressions_mutator(Node *nod
*** 2365,2385 ****
  		case T_OpExpr:
  			{
  				OpExpr	   *expr = (OpExpr *) node;
! 				List	   *args;
  				Expr	   *simple;
  				OpExpr	   *newexpr;
  
  				/*
- 				 * Reduce constants in the OpExpr's arguments.  We know args
- 				 * is either NIL or a List node, so we can call
- 				 * expression_tree_mutator directly rather than recursing to
- 				 * self.
- 				 */
- 				args = (List *) expression_tree_mutator((Node *) expr->args,
- 											  eval_const_expressions_mutator,
- 														(void *) context);
- 
- 				/*
  				 * Need to get OID of underlying function.	Okay to scribble
  				 * on input to this extent.
  				 */
--- 2347,2357 ----
  		case T_OpExpr:
  			{
  				OpExpr	   *expr = (OpExpr *) node;
! 				List	   *args = expr->args;
  				Expr	   *simple;
  				OpExpr	   *newexpr;
  
  				/*
  				 * Need to get OID of underlying function.	Okay to scribble
  				 * on input to this extent.
  				 */
*************** eval_const_expressions_mutator(Node *nod
*** 2387,2393 ****
  
  				/*
  				 * Code for op/func reduction is pretty bulky, so split it out
! 				 * as a separate function.
  				 */
  				simple = simplify_function((Expr *) expr,
  										   expr->opfuncid,
--- 2359,2366 ----
  
  				/*
  				 * Code for op/func reduction is pretty bulky, so split it out
! 				 * as a separate function. This function also mutates the
! 				 * argument list.
  				 */
  				simple = simplify_function((Expr *) expr,
  										   expr->opfuncid,
*************** eval_const_expressions_mutator(Node *nod
*** 2395,2401 ****
  										   expr->opcollid,
  										   expr->inputcollid,
  										   &args,
! 										   false, true, context);
  				if (simple)		/* successfully simplified it */
  					return (Node *) simple;
  
--- 2368,2374 ----
  										   expr->opcollid,
  										   expr->inputcollid,
  										   &args,
! 										   true, true, context);
  				if (simple)		/* successfully simplified it */
  					return (Node *) simple;
  
*************** eval_const_expressions_mutator(Node *nod
*** 2668,2674 ****
  		case T_CoerceViaIO:
  			{
  				CoerceViaIO *expr = (CoerceViaIO *) node;
- 				Expr	   *arg;
  				List	   *args;
  				Oid			outfunc;
  				bool		outtypisvarlena;
--- 2641,2646 ----
*************** eval_const_expressions_mutator(Node *nod
*** 2677,2688 ****
  				Expr	   *simple;
  				CoerceViaIO *newexpr;
  
! 				/*
! 				 * Reduce constants in the CoerceViaIO's argument.
! 				 */
! 				arg = (Expr *) eval_const_expressions_mutator((Node *) expr->arg,
! 															  context);
! 				args = list_make1(arg);
  
  				/*
  				 * CoerceViaIO represents calling the source type's output
--- 2649,2655 ----
  				Expr	   *simple;
  				CoerceViaIO *newexpr;
  
! 				args = list_make1(expr->arg);
  
  				/*
  				 * CoerceViaIO represents calling the source type's output
*************** eval_const_expressions_mutator(Node *nod
*** 2693,2710 ****
  				 * Note that the coercion functions are assumed not to care
  				 * about input collation, so we just pass InvalidOid for that.
  				 */
! 				getTypeOutputInfo(exprType((Node *) arg),
  								  &outfunc, &outtypisvarlena);
  				getTypeInputInfo(expr->resulttype,
  								 &infunc, &intypioparam);
  
  				simple = simplify_function(NULL,
  										   outfunc,
  										   CSTRINGOID, -1,
  										   InvalidOid,
  										   InvalidOid,
  										   &args,
! 										   false, true, context);
  				if (simple)		/* successfully simplified output fn */
  				{
  					/*
--- 2660,2678 ----
  				 * Note that the coercion functions are assumed not to care
  				 * about input collation, so we just pass InvalidOid for that.
  				 */
! 				getTypeOutputInfo(exprType((Node *) expr->arg),
  								  &outfunc, &outtypisvarlena);
  				getTypeInputInfo(expr->resulttype,
  								 &infunc, &intypioparam);
  
+ 				/* Mutate the argument and try to simplify */
  				simple = simplify_function(NULL,
  										   outfunc,
  										   CSTRINGOID, -1,
  										   InvalidOid,
  										   InvalidOid,
  										   &args,
! 										   true, true, context);
  				if (simple)		/* successfully simplified output fn */
  				{
  					/*
*************** eval_const_expressions_mutator(Node *nod
*** 2745,2751 ****
  				 * possibly-simplified argument.
  				 */
  				newexpr = makeNode(CoerceViaIO);
! 				newexpr->arg = arg;
  				newexpr->resulttype = expr->resulttype;
  				newexpr->resultcollid = expr->resultcollid;
  				newexpr->coerceformat = expr->coerceformat;
--- 2713,2719 ----
  				 * possibly-simplified argument.
  				 */
  				newexpr = makeNode(CoerceViaIO);
! 				newexpr->arg = linitial(args);
  				newexpr->resulttype = expr->resulttype;
  				newexpr->resultcollid = expr->resultcollid;
  				newexpr->coerceformat = expr->coerceformat;
*************** simplify_boolean_equality(Oid opno, List
*** 3584,3590 ****
   * Inputs are the original expression (can be NULL), function OID, actual
   * result type OID (which is needed for polymorphic functions), result typmod,
   * result collation, the input collation to use for the function, the
!  * pre-simplified argument list, and some flags; also the context data for
   * eval_const_expressions.	In common cases, several of the arguments could be
   * derived from the original expression.  Sending them separately avoids
   * duplicating NodeTag-specific knowledge, and it's necessary for CoerceViaIO.
--- 3552,3558 ----
   * Inputs are the original expression (can be NULL), function OID, actual
   * result type OID (which is needed for polymorphic functions), result typmod,
   * result collation, the input collation to use for the function, the
!  * un-simplified argument list, and some flags; also the context data for
   * eval_const_expressions.	In common cases, several of the arguments could be
   * derived from the original expression.  Sending them separately avoids
   * duplicating NodeTag-specific knowledge, and it's necessary for CoerceViaIO.
*************** simplify_boolean_equality(Oid opno, List
*** 3603,3615 ****
  static Expr *
  simplify_function(Expr *oldexpr, Oid funcid,
  				  Oid result_type, int32 result_typmod, Oid result_collid,
! 				  Oid input_collid, List **args,
! 				  bool has_named_args,
! 				  bool allow_inline,
  				  eval_const_expressions_context *context)
  {
  	HeapTuple	func_tuple;
  	Expr	   *newexpr;
  	Oid			transform;
  
  	/*
--- 3571,3584 ----
  static Expr *
  simplify_function(Expr *oldexpr, Oid funcid,
  				  Oid result_type, int32 result_typmod, Oid result_collid,
! 				  Oid input_collid, List **args_p,
! 				  bool mutate_args, bool allow_inline,
  				  eval_const_expressions_context *context)
  {
  	HeapTuple	func_tuple;
  	Expr	   *newexpr;
+ 	ListCell   *lc;
+ 	List	   *args = *args_p;
  	Oid			transform;
  
  	/*
*************** simplify_function(Expr *oldexpr, Oid fun
*** 3624,3641 ****
  	if (!HeapTupleIsValid(func_tuple))
  		elog(ERROR, "cache lookup failed for function %u", funcid);
  
! 	/*
! 	 * While we have the tuple, reorder named arguments and add default
! 	 * arguments if needed.
! 	 */
! 	if (has_named_args)
! 		*args = reorder_function_arguments(*args, result_type, func_tuple,
! 										   context);
! 	else if (((Form_pg_proc) GETSTRUCT(func_tuple))->pronargs > list_length(*args))
! 		*args = add_function_defaults(*args, result_type, func_tuple, context);
  
  	newexpr = evaluate_function(funcid, result_type, result_typmod,
! 								result_collid, input_collid, *args,
  								func_tuple, context);
  
  	/*
--- 3593,3622 ----
  	if (!HeapTupleIsValid(func_tuple))
  		elog(ERROR, "cache lookup failed for function %u", funcid);
  
! 	if (mutate_args)
! 	{
! 		if (oldexpr && IsA(oldexpr, FuncExpr))
! 			/*
! 			 * Reorder named arguments and add defaults if needed. Returns a
! 			 * copied list, so we can mutate it later.
! 			 */
! 			args = simplify_copy_function_arguments(args, result_type, func_tuple);
! 		else
! 			/* Copy argument list before we start mutating it. */
! 			args = list_copy(args);
! 
! 		/* Reduce constants in the expression's arguments */
! 		foreach(lc, args)
! 		{
! 			Node	   *arg = (Node *) lfirst(lc);
! 
! 			arg = eval_const_expressions_mutator(arg, context);
! 			lfirst(lc) = arg;
! 		}
! 	}
  
  	newexpr = evaluate_function(funcid, result_type, result_typmod,
! 								result_collid, input_collid, args,
  								func_tuple, context);
  
  	/*
*************** simplify_function(Expr *oldexpr, Oid fun
*** 3674,3702 ****
  
  	if (!newexpr && allow_inline)
  		newexpr = inline_function(funcid, result_type, result_collid,
! 								  input_collid, *args,
  								  func_tuple, context);
  
  	ReleaseSysCache(func_tuple);
  
  	return newexpr;
  }
  
  /*
   * reorder_function_arguments: convert named-notation args to positional args
   *
   * This function also inserts default argument values as needed, since it's
   * impossible to form a truly valid positional call without that.
   */
  static List *
! reorder_function_arguments(List *args, Oid result_type, HeapTuple func_tuple,
! 						   eval_const_expressions_context *context)
  {
  	Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
  	int			pronargs = funcform->pronargs;
  	int			nargsprovided = list_length(args);
  	Node	   *argarray[FUNC_MAX_ARGS];
- 	Bitmapset  *defargnumbers;
  	ListCell   *lc;
  	int			i;
  
--- 3655,3734 ----
  
  	if (!newexpr && allow_inline)
  		newexpr = inline_function(funcid, result_type, result_collid,
! 								  input_collid, args,
  								  func_tuple, context);
  
  	ReleaseSysCache(func_tuple);
  
+ 	/* Argument processing done, give it back to the caller */
+ 	*args_p = args;
+ 
  	return newexpr;
  }
  
  /*
+  * This function converts a named-notation argument list into positional
+  * notation while adding any needed default argument expressions
+  *
+  * A copy of the argument list is returned, the original is not modified.
+  */
+ static List *
+ simplify_copy_function_arguments(List *old_args, Oid result_type,
+ 							HeapTuple func_tuple)
+ {
+ 	List	   *args = NIL;
+ 	ListCell   *lc;
+ 	bool		has_named_args = false;
+ 	int			args_before;
+ 
+ 	/* Do we need to reorder named arguments? */
+ 	foreach(lc, old_args)
+ 	{
+ 		Node	   *arg = (Node *) lfirst(lc);
+ 
+ 		if (IsA(arg, NamedArgExpr))
+ 		{
+ 			has_named_args = true;
+ 			break;
+ 		}
+ 	}
+ 
+ 	args_before = list_length(old_args);
+ 
+ 	if (has_named_args)
+ 	{
+ 		/* Reorder named arguments and add default arguments if needed. */
+ 		args = reorder_function_arguments(old_args, result_type, func_tuple);
+ 	}
+ 	else
+ 	{
+ 		args = list_copy(old_args);
+ 
+ 		/* Append missing default arguments to the list */
+ 		if (((Form_pg_proc) GETSTRUCT(func_tuple))->pronargs > list_length(args))
+ 			args = add_function_defaults(args, result_type, func_tuple);
+ 	}
+ 
+ 	/* If we added any default args, they may need casts */
+ 	if (list_length(args) != args_before)
+ 		recheck_cast_function_args(args, result_type, func_tuple);
+ 
+ 	return args;
+ }
+ 
+ /*
   * reorder_function_arguments: convert named-notation args to positional args
   *
   * This function also inserts default argument values as needed, since it's
   * impossible to form a truly valid positional call without that.
   */
  static List *
! reorder_function_arguments(List *args, Oid result_type, HeapTuple func_tuple)
  {
  	Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
  	int			pronargs = funcform->pronargs;
  	int			nargsprovided = list_length(args);
  	Node	   *argarray[FUNC_MAX_ARGS];
  	ListCell   *lc;
  	int			i;
  
*************** reorder_function_arguments(List *args, O
*** 3730,3736 ****
  	 * Fetch default expressions, if needed, and insert into array at proper
  	 * locations (they aren't necessarily consecutive or all used)
  	 */
- 	defargnumbers = NULL;
  	if (nargsprovided < pronargs)
  	{
  		List	   *defaults = fetch_function_defaults(func_tuple);
--- 3762,3767 ----
*************** reorder_function_arguments(List *args, O
*** 3739,3748 ****
  		foreach(lc, defaults)
  		{
  			if (argarray[i] == NULL)
- 			{
  				argarray[i] = (Node *) lfirst(lc);
- 				defargnumbers = bms_add_member(defargnumbers, i);
- 			}
  			i++;
  		}
  	}
--- 3770,3776 ----
*************** reorder_function_arguments(List *args, O
*** 3755,3786 ****
  		args = lappend(args, argarray[i]);
  	}
  
- 	/* Recheck argument types and add casts if needed */
- 	recheck_cast_function_args(args, result_type, func_tuple);
- 
- 	/*
- 	 * Lastly, we have to recursively simplify the defaults we just added (but
- 	 * don't recurse on the args passed in, as we already did those). This
- 	 * isn't merely an optimization, it's *necessary* since there could be
- 	 * functions with named or defaulted arguments down in there.
- 	 *
- 	 * Note that we do this last in hopes of simplifying any typecasts that
- 	 * were added by recheck_cast_function_args --- there shouldn't be any new
- 	 * casts added to the explicit arguments, but casts on the defaults are
- 	 * possible.
- 	 */
- 	if (defargnumbers != NULL)
- 	{
- 		i = 0;
- 		foreach(lc, args)
- 		{
- 			if (bms_is_member(i, defargnumbers))
- 				lfirst(lc) = eval_const_expressions_mutator((Node *) lfirst(lc),
- 															context);
- 			i++;
- 		}
- 	}
- 
  	return args;
  }
  
--- 3783,3788 ----
*************** reorder_function_arguments(List *args, O
*** 3791,3810 ****
   * and so we know we just need to add defaults at the end.
   */
  static List *
! add_function_defaults(List *args, Oid result_type, HeapTuple func_tuple,
! 					  eval_const_expressions_context *context)
  {
  	Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
- 	int			nargsprovided = list_length(args);
  	List	   *defaults;
  	int			ndelete;
- 	ListCell   *lc;
  
  	/* Get all the default expressions from the pg_proc tuple */
  	defaults = fetch_function_defaults(func_tuple);
  
  	/* Delete any unused defaults from the list */
! 	ndelete = nargsprovided + list_length(defaults) - funcform->pronargs;
  	if (ndelete < 0)
  		elog(ERROR, "not enough default arguments");
  	while (ndelete-- > 0)
--- 3793,3809 ----
   * and so we know we just need to add defaults at the end.
   */
  static List *
! add_function_defaults(List *args, Oid result_type, HeapTuple func_tuple)
  {
  	Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple);
  	List	   *defaults;
  	int			ndelete;
  
  	/* Get all the default expressions from the pg_proc tuple */
  	defaults = fetch_function_defaults(func_tuple);
  
  	/* 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)
*************** add_function_defaults(List *args, Oid re
*** 3813,3840 ****
  	/* And form the combined argument list */
  	args = list_concat(args, defaults);
  
- 	/* Recheck argument types and add casts if needed */
- 	recheck_cast_function_args(args, result_type, func_tuple);
- 
- 	/*
- 	 * Lastly, we have to recursively simplify the defaults we just added (but
- 	 * don't recurse on the args passed in, as we already did those). This
- 	 * isn't merely an optimization, it's *necessary* since there could be
- 	 * functions with named or defaulted arguments down in there.
- 	 *
- 	 * Note that we do this last in hopes of simplifying any typecasts that
- 	 * were added by recheck_cast_function_args --- there shouldn't be any new
- 	 * casts added to the explicit arguments, but casts on the defaults are
- 	 * possible.
- 	 */
- 	foreach(lc, args)
- 	{
- 		if (nargsprovided-- > 0)
- 			continue;			/* skip original arg positions */
- 		lfirst(lc) = eval_const_expressions_mutator((Node *) lfirst(lc),
- 													context);
- 	}
- 
  	return args;
  }
  
--- 3812,3817 ----
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to