diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 2d3ec22407..5ce0079a12 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -108,6 +108,7 @@ static bool contain_volatile_functions_not_nextval_walker(Node *node, void *cont static bool max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context); static bool contain_nonstrict_functions_walker(Node *node, void *context); +static bool contain_inlinable_functions_walker(Node *node, void *context); static bool contain_context_dependent_node(Node *clause); static bool contain_context_dependent_node_walker(Node *node, int *flags); static bool contain_leaked_vars_walker(Node *node, void *context); @@ -1218,6 +1219,63 @@ contain_nonstrict_functions_walker(Node *node, void *context) context); } +/***************************************************************************** + * Check clauses for inline-able functions + *****************************************************************************/ + +bool +contain_inlinable_functions(Node *node) +{ + return contain_inlinable_functions_walker(node, NULL); +} + +/* + * can_inline_function - checks if a function is inline-able + */ +static bool +can_inline_function(HeapTuple func_tuple) +{ + Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple); + + /* + * Nope if the function is not SQL-language or has other showstopper + * properties. (The prokind and nargs checks are just paranoia.) + */ + return funcform->prolang == SQLlanguageId && + funcform->prokind == PROKIND_FUNCTION && + !funcform->prosecdef && !funcform->proretset && + funcform->prorettype != RECORDOID && + heap_attisnull(func_tuple, Anum_pg_proc_proconfig, NULL); +} + +static bool +can_inline_function_checker(Oid funcid, void *context) +{ + HeapTuple func_tuple; + bool result; + + func_tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid)); + if (!HeapTupleIsValid(func_tuple)) + elog(ERROR, "cache lookup failed for function %u", funcid); + + result = can_inline_function(func_tuple); + ReleaseSysCache(func_tuple); + + return result; +} + +static bool +contain_inlinable_functions_walker(Node *node, void *context) +{ + if (node == NULL) + return false; + if (check_functions_in_node(node, can_inline_function_checker, context)) + return true; + + return expression_tree_walker(node, contain_inlinable_functions_walker, + context); +} + /***************************************************************************** * Check clauses for context-dependent nodes *****************************************************************************/ @@ -4022,7 +4080,8 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod, Assert(newexpr != (Expr *) &fexpr); } - if (!newexpr && allow_non_const) + if (!newexpr && allow_non_const && + can_inline_function(func_tuple)) newexpr = inline_function(funcid, result_type, result_collid, input_collid, args, funcvariadic, func_tuple, context); @@ -4415,16 +4474,11 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid, int i; /* - * Forget it if the function is not SQL-language or has other showstopper - * properties. (The prokind and nargs checks are just paranoia.) + * Caller should already have checked whether the function can be inlined + * using can_function_inline(). */ - if (funcform->prolang != SQLlanguageId || - funcform->prokind != PROKIND_FUNCTION || - funcform->prosecdef || - funcform->proretset || - funcform->prorettype == RECORDOID || - !heap_attisnull(func_tuple, Anum_pg_proc_proconfig, NULL) || - funcform->pronargs != list_length(args)) + + if (funcform->pronargs != list_length(args)) return NULL; /* Check for recursive function, and give up trying to expand if so */ diff --git a/src/include/optimizer/optimizer.h b/src/include/optimizer/optimizer.h index 5283995df8..4954657147 100644 --- a/src/include/optimizer/optimizer.h +++ b/src/include/optimizer/optimizer.h @@ -130,6 +130,7 @@ extern Expr *canonicalize_qual(Expr *qual, bool is_check); extern bool contain_mutable_functions(Node *clause); extern bool contain_volatile_functions(Node *clause); extern bool contain_volatile_functions_not_nextval(Node *clause); +extern bool contain_inlinable_functions(Node *node); extern Node *eval_const_expressions(PlannerInfo *root, Node *node); diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 5acf604f63..07d468ce5d 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -5758,9 +5758,10 @@ exec_eval_expr(PLpgSQL_execstate *estate, Form_pg_attribute attr; /* - * If first time through, create a plan for this expression. + * Create a plan for this expression, if first time through or the + * existing plan is no longer valid. */ - if (expr->plan == NULL) + if (expr->plan == NULL || !SPI_plan_is_valid(expr->plan)) exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK, true); /* @@ -6076,7 +6077,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, { ExprContext *econtext = estate->eval_econtext; LocalTransactionId curlxid = MyProc->lxid; - CachedPlan *cplan; void *save_setup_arg; bool need_snapshot; MemoryContext oldcontext; @@ -6094,28 +6094,39 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, return false; /* - * Revalidate cached plan, so that we will notice if it became stale. (We - * need to hold a refcount while using the plan, anyway.) If replanning - * is needed, do that work in the eval_mcontext. + * Revalidate cached plan if one would have been used (due to inline-able) + * functions being found in the expression), so that we will notice if it + * became stale. (We need to hold a refcount while using the plan, + * anyway.) If replanning is needed, do that work in the eval_mcontext. */ - oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); - cplan = SPI_plan_get_cached_plan(expr->plan); - MemoryContextSwitchTo(oldcontext); - - /* - * We can't get a failure here, because the number of CachedPlanSources in - * the SPI plan can't change from what exec_simple_check_plan saw; it's a - * property of the raw parsetree generated from the query text. - */ - Assert(cplan != NULL); - - /* If it got replanned, update our copy of the simple expression */ - if (cplan->generation != expr->expr_simple_generation) + if (expr->expr_contains_inline_func) { - exec_save_simple_expr(expr, cplan); - /* better recheck r/w safety, as it could change due to inlining */ - if (expr->rwparam >= 0) - exec_check_rw_parameter(expr, expr->rwparam); + CachedPlan *cplan; + + oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); + cplan = SPI_plan_get_cached_plan(expr->plan); + MemoryContextSwitchTo(oldcontext); + + /* + * We can't get a failure here, because the number of CachedPlanSources in + * the SPI plan can't change from what exec_simple_check_plan saw; it's a + * property of the raw parsetree generated from the query text. + */ + Assert(cplan != NULL); + + /* If it got replanned, update our copy of the simple expression */ + if (cplan->generation != expr->expr_simple_generation) + { + exec_save_simple_expr(expr, cplan); + /* better recheck r/w safety, as it could change due to inlining */ + if (expr->rwparam >= 0) + exec_check_rw_parameter(expr, expr->rwparam); + } + + /* + * Now we can release our refcount on the cached plan. + */ + ReleaseCachedPlan(cplan, true); } /* @@ -6192,11 +6203,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, MemoryContextSwitchTo(oldcontext); - /* - * Now we can release our refcount on the cached plan. - */ - ReleaseCachedPlan(cplan, true); - /* * That's it. */ @@ -7893,6 +7899,8 @@ get_cast_hashentry(PLpgSQL_execstate *estate, * exec_simple_check_plan - Check if a plan is simple enough to * be evaluated by ExecEvalExpr() instead * of SPI. + * + * If it is, set expr->expr_simple_expr. * ---------- */ static void @@ -7901,8 +7909,7 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) List *plansources; CachedPlanSource *plansource; Query *query; - CachedPlan *cplan; - MemoryContext oldcontext; + Expr *tle_expr; /* * Initialize to "not simple". @@ -7972,23 +7979,55 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) return; /* - * OK, we can treat it as a simple plan. - * - * Get the generic plan for the query. If replanning is needed, do that - * work in the eval_mcontext. + * We have a simple expression. Although, if it contains inline-able + * SQL functions, better pass it through the planner to get a simpler + * plan. */ - oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); - cplan = SPI_plan_get_cached_plan(expr->plan); - MemoryContextSwitchTo(oldcontext); + tle_expr = castNode(TargetEntry, linitial(query->targetList))->expr; + if (contain_inlinable_functions((Node *) tle_expr)) + { + CachedPlan *cplan = NULL; + MemoryContext oldcontext; - /* Can't fail, because we checked for a single CachedPlanSource above */ - Assert(cplan != NULL); + expr->expr_contains_inline_func = true; - /* Share the remaining work with replan code path */ - exec_save_simple_expr(expr, cplan); + /* + * Get the generic plan for the query. If replanning is needed, do + * that work in the eval_mcontext. + */ + oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); + cplan = SPI_plan_get_cached_plan(expr->plan); + MemoryContextSwitchTo(oldcontext); - /* Release our plan refcount */ - ReleaseCachedPlan(cplan, true); + /* + * Can't fail, because we checked for a single CachedPlanSource + * above. + */ + Assert(cplan != NULL); + + exec_save_simple_expr(expr, cplan); + + /* Release our plan refcount */ + ReleaseCachedPlan(cplan, true); + } + else + { + /* + * Save the simple expression, and initialize state to "not valid in + * current transaction". + */ + expr->expr_simple_expr = tle_expr; + expr->expr_simple_generation = plansource->generation; + expr->expr_simple_state = NULL; + expr->expr_simple_in_use = false; + expr->expr_simple_lxid = InvalidLocalTransactionId; + /* Also stash away the expression result type */ + expr->expr_simple_type = exprType((Node *) tle_expr); + expr->expr_simple_typmod = exprTypmod((Node *) tle_expr); + /* We also want to remember if it is immutable or not */ + expr->expr_simple_mutable = contain_mutable_functions((Node *) tle_expr); + expr->expr_contains_inline_func = false; + } } /* diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index b599f67fc5..0e02212135 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -245,6 +245,7 @@ typedef struct PLpgSQL_expr ExprState *expr_simple_state; /* eval tree for expr_simple_expr */ bool expr_simple_in_use; /* true if eval tree is active */ LocalTransactionId expr_simple_lxid; + bool expr_contains_inline_func; } PLpgSQL_expr; /* diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index cd2c79f4d5..3abbc7bc57 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -4289,12 +4289,10 @@ end $$; select fail(); ERROR: division by zero -CONTEXT: SQL statement "SELECT 1/0" -PL/pgSQL function fail() line 3 at RETURN +CONTEXT: PL/pgSQL function fail() line 3 at RETURN select fail(); ERROR: division by zero -CONTEXT: SQL statement "SELECT 1/0" -PL/pgSQL function fail() line 3 at RETURN +CONTEXT: PL/pgSQL function fail() line 3 at RETURN drop function fail(); -- Test handling of string literals. set standard_conforming_strings = off;