diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile index af9f6fb209..2057e8a99c 100644 --- a/src/pl/plpgsql/src/Makefile +++ b/src/pl/plpgsql/src/Makefile @@ -27,7 +27,7 @@ DATA = plpgsql.control plpgsql--1.0.sql plpgsql--unpackaged--1.0.sql REGRESS_OPTS = --dbname=$(PL_TESTDB) REGRESS = plpgsql_call plpgsql_control plpgsql_domain plpgsql_record \ - plpgsql_cache plpgsql_transaction plpgsql_trap \ + plpgsql_cache plpgsql_simple plpgsql_transaction plpgsql_trap \ plpgsql_trigger plpgsql_varprops # where to find gen_keywordlist.pl and subsidiary files diff --git a/src/pl/plpgsql/src/expected/plpgsql_simple.out b/src/pl/plpgsql/src/expected/plpgsql_simple.out new file mode 100644 index 0000000000..7b22e60f19 --- /dev/null +++ b/src/pl/plpgsql/src/expected/plpgsql_simple.out @@ -0,0 +1,120 @@ +-- +-- Tests for plpgsql's handling of "simple" expressions +-- +-- Check that changes to an inline-able function are handled correctly +create function simplesql(int) returns int language sql +as 'select $1'; +create function simplecaller() returns int language plpgsql +as $$ +declare + sum int := 0; +begin + for n in 1..10 loop + sum := sum + simplesql(n); + if n = 5 then + create or replace function simplesql(int) returns int language sql + as 'select $1 + 100'; + end if; + end loop; + return sum; +end$$; +select simplecaller(); + simplecaller +-------------- + 555 +(1 row) + +-- Check that changes in search path are dealt with correctly +create schema simple1; +create function simple1.simpletarget(int) returns int language plpgsql +as $$begin return $1; end$$; +create function simpletarget(int) returns int language plpgsql +as $$begin return $1 + 100; end$$; +create or replace function simplecaller() returns int language plpgsql +as $$ +declare + sum int := 0; +begin + for n in 1..10 loop + sum := sum + simpletarget(n); + if n = 5 then + set local search_path = 'simple1'; + end if; + end loop; + return sum; +end$$; +select simplecaller(); + simplecaller +-------------- + 555 +(1 row) + +-- try it with non-volatile functions, too +alter function simple1.simpletarget(int) immutable; +alter function simpletarget(int) immutable; +select simplecaller(); + simplecaller +-------------- + 555 +(1 row) + +-- make sure flushing local caches changes nothing +\c - +select simplecaller(); + simplecaller +-------------- + 555 +(1 row) + +-- Check case where first attempt to determine if it's simple fails +create function simplesql() returns int language sql +as $$select 1 / 0$$; +create or replace function simplecaller() returns int language plpgsql +as $$ +declare x int; +begin + select simplesql() into x; + return x; +end$$; +select simplecaller(); -- division by zero occurs during simple-expr check +ERROR: division by zero +CONTEXT: SQL function "simplesql" during inlining +SQL statement "select simplesql()" +PL/pgSQL function simplecaller() line 4 at SQL statement +create or replace function simplesql() returns int language sql +as $$select 2 + 2$$; +select simplecaller(); + simplecaller +-------------- + 4 +(1 row) + +-- Check case where called function changes from non-SRF to SRF (bug #18497) +create or replace function simplecaller() returns int language plpgsql +as $$ +declare x int; +begin + x := simplesql(); + return x; +end$$; +select simplecaller(); + simplecaller +-------------- + 4 +(1 row) + +drop function simplesql(); +create function simplesql() returns setof int language sql +as $$select 22 + 22$$; +select simplecaller(); + simplecaller +-------------- + 44 +(1 row) + +select simplecaller(); + simplecaller +-------------- + 44 +(1 row) + diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 9fc92a77bf..bfb70135d2 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -339,6 +339,7 @@ static void exec_prepare_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr, int cursorOptions, bool keepplan); static void exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr); +static bool exec_is_simple_query(PLpgSQL_expr *expr); static void exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan); static void exec_check_rw_parameter(PLpgSQL_expr *expr, int target_dno); static bool contains_target_param(Node *node, int *target_dno); @@ -6216,6 +6217,19 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate, */ Assert(cplan != NULL); + /* + * However, the plan might now be non-simple, in edge-case scenarios such + * as a non-SRF having been replaced with a SRF. + */ + if (!exec_is_simple_query(expr)) + { + /* Release SPI_plan_get_cached_plan's refcount */ + ReleaseCachedPlan(cplan, true); + /* Mark expression as non-simple, and fail */ + expr->expr_simple_expr = NULL; + return false; + } + /* If it got replanned, update our copy of the simple expression */ if (cplan->generation != expr->expr_simple_generation) { @@ -8023,9 +8037,6 @@ get_cast_hashentry(PLpgSQL_execstate *estate, static void exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) { - List *plansources; - CachedPlanSource *plansource; - Query *query; CachedPlan *cplan; MemoryContext oldcontext; @@ -8040,31 +8051,66 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) * called immediately after creating the CachedPlanSource, we need not * worry about the query being stale. */ + if (!exec_is_simple_query(expr)) + return; /* - * We can only test queries that resulted in exactly one CachedPlanSource + * Get the generic plan for the query. If replanning is needed, do that + * work in the eval_mcontext. (Note that replanning could throw an error, + * in which case the expr is left marked "not simple", which is fine.) + */ + oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); + cplan = SPI_plan_get_cached_plan(expr->plan); + MemoryContextSwitchTo(oldcontext); + + /* Can't fail, because we checked for a single CachedPlanSource above */ + Assert(cplan != NULL); + + /* Share the remaining work with replan code path */ + exec_save_simple_expr(expr, cplan); + + /* Release our plan refcount */ + ReleaseCachedPlan(cplan, true); +} + +/* + * exec_is_simple_query - precheck a query tree to see if it might be simple + * + * Check the analyzed-and-rewritten form of a query to see if we will be + * able to treat it as a simple expression. It is caller's responsibility + * that the CachedPlanSource be up-to-date. + */ +static bool +exec_is_simple_query(PLpgSQL_expr *expr) +{ + List *plansources; + CachedPlanSource *plansource; + Query *query; + + /* + * We can only test queries that resulted in exactly one CachedPlanSource. */ plansources = SPI_plan_get_plan_sources(expr->plan); if (list_length(plansources) != 1) - return; + return false; plansource = (CachedPlanSource *) linitial(plansources); /* * 1. There must be one single querytree. */ if (list_length(plansource->query_list) != 1) - return; + return false; query = (Query *) linitial(plansource->query_list); /* - * 2. It must be a plain SELECT query without any input tables + * 2. It must be a plain SELECT query without any input tables. */ if (!IsA(query, Query)) - return; + return false; if (query->commandType != CMD_SELECT) - return; + return false; if (query->rtable != NIL) - return; + return false; /* * 3. Can't have any subplans, aggregates, qual clauses either. (These @@ -8088,33 +8134,18 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) query->limitOffset || query->limitCount || query->setOperations) - return; + return false; /* - * 4. The query must have a single attribute as result + * 4. The query must have a single attribute as result. */ if (list_length(query->targetList) != 1) - return; + return false; /* * 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. (Note that replanning could throw an error, - * in which case the expr is left marked "not simple", which is fine.) */ - oldcontext = MemoryContextSwitchTo(get_eval_mcontext(estate)); - cplan = SPI_plan_get_cached_plan(expr->plan); - MemoryContextSwitchTo(oldcontext); - - /* Can't fail, because we checked for a single CachedPlanSource above */ - Assert(cplan != NULL); - - /* Share the remaining work with replan code path */ - exec_save_simple_expr(expr, cplan); - - /* Release our plan refcount */ - ReleaseCachedPlan(cplan, true); + return true; } /* diff --git a/src/pl/plpgsql/src/sql/plpgsql_simple.sql b/src/pl/plpgsql/src/sql/plpgsql_simple.sql new file mode 100644 index 0000000000..143bf09dce --- /dev/null +++ b/src/pl/plpgsql/src/sql/plpgsql_simple.sql @@ -0,0 +1,104 @@ +-- +-- Tests for plpgsql's handling of "simple" expressions +-- + +-- Check that changes to an inline-able function are handled correctly +create function simplesql(int) returns int language sql +as 'select $1'; + +create function simplecaller() returns int language plpgsql +as $$ +declare + sum int := 0; +begin + for n in 1..10 loop + sum := sum + simplesql(n); + if n = 5 then + create or replace function simplesql(int) returns int language sql + as 'select $1 + 100'; + end if; + end loop; + return sum; +end$$; + +select simplecaller(); + + +-- Check that changes in search path are dealt with correctly +create schema simple1; + +create function simple1.simpletarget(int) returns int language plpgsql +as $$begin return $1; end$$; + +create function simpletarget(int) returns int language plpgsql +as $$begin return $1 + 100; end$$; + +create or replace function simplecaller() returns int language plpgsql +as $$ +declare + sum int := 0; +begin + for n in 1..10 loop + sum := sum + simpletarget(n); + if n = 5 then + set local search_path = 'simple1'; + end if; + end loop; + return sum; +end$$; + +select simplecaller(); + +-- try it with non-volatile functions, too +alter function simple1.simpletarget(int) immutable; +alter function simpletarget(int) immutable; + +select simplecaller(); + +-- make sure flushing local caches changes nothing +\c - + +select simplecaller(); + + +-- Check case where first attempt to determine if it's simple fails + +create function simplesql() returns int language sql +as $$select 1 / 0$$; + +create or replace function simplecaller() returns int language plpgsql +as $$ +declare x int; +begin + select simplesql() into x; + return x; +end$$; + +select simplecaller(); -- division by zero occurs during simple-expr check + +create or replace function simplesql() returns int language sql +as $$select 2 + 2$$; + +select simplecaller(); + + +-- Check case where called function changes from non-SRF to SRF (bug #18497) + +create or replace function simplecaller() returns int language plpgsql +as $$ +declare x int; +begin + x := simplesql(); + return x; +end$$; + +select simplecaller(); + +drop function simplesql(); + +create function simplesql() returns setof int language sql +as $$select 22 + 22$$; + +select simplecaller(); + +select simplecaller();