mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-10-02 22:56:50 +02:00
When replanning a plpgsql "simple expression", check it's still simple.
The previous coding here assumed that we didn't need to recheck any of the querytree tests made in exec_simple_check_plan(). I think we supposed that those properties were fully determined by the syntax of the source text and hence couldn't change. That is true for most of them, but at least hasTargetSRFs and hasAggs can change by dint of forcibly dropping an originally-referenced function and recreating it with new properties. That leads to "unexpected plan node type" or similar failures. These tests are pretty cheap compared to the cost of replanning, so rather than sweat over exactly which properties need to be rechecked, let's just recheck them all. Hence, factor out those tests into a new function exec_is_simple_query(), and rearrange callers as needed. A second problem in the same area was that if we failed during replanning or during exec_save_simple_expr(), we'd potentially leave behind now-dangling pointers to the old simple expression, potentially resulting in crashes later. To fix, clear those pointers before replanning. The v12 code looks quite different in this area but still has the bug about needing to recheck query simplicity. I chose to back-patch all of the plpgsql_simple.sql test script, which formerly didn't exist in this branch. Per bug #18497 from Nikita Kalinin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18497-fe93b6da82ce31d4@postgresql.org
This commit is contained in:
parent
a5662ba345
commit
ec210914c7
@ -27,7 +27,7 @@ DATA = plpgsql.control plpgsql--1.0.sql plpgsql--unpackaged--1.0.sql
|
|||||||
REGRESS_OPTS = --dbname=$(PL_TESTDB)
|
REGRESS_OPTS = --dbname=$(PL_TESTDB)
|
||||||
|
|
||||||
REGRESS = plpgsql_call plpgsql_control plpgsql_domain plpgsql_record \
|
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
|
plpgsql_trigger plpgsql_varprops
|
||||||
|
|
||||||
# where to find gen_keywordlist.pl and subsidiary files
|
# where to find gen_keywordlist.pl and subsidiary files
|
||||||
|
120
src/pl/plpgsql/src/expected/plpgsql_simple.out
Normal file
120
src/pl/plpgsql/src/expected/plpgsql_simple.out
Normal file
@ -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)
|
||||||
|
|
@ -339,6 +339,7 @@ static void exec_prepare_plan(PLpgSQL_execstate *estate,
|
|||||||
PLpgSQL_expr *expr, int cursorOptions,
|
PLpgSQL_expr *expr, int cursorOptions,
|
||||||
bool keepplan);
|
bool keepplan);
|
||||||
static void exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr);
|
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_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan);
|
||||||
static void exec_check_rw_parameter(PLpgSQL_expr *expr, int target_dno);
|
static void exec_check_rw_parameter(PLpgSQL_expr *expr, int target_dno);
|
||||||
static bool contains_target_param(Node *node, 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);
|
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 it got replanned, update our copy of the simple expression */
|
||||||
if (cplan->generation != expr->expr_simple_generation)
|
if (cplan->generation != expr->expr_simple_generation)
|
||||||
{
|
{
|
||||||
@ -8023,9 +8037,6 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
|
|||||||
static void
|
static void
|
||||||
exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
|
exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
|
||||||
{
|
{
|
||||||
List *plansources;
|
|
||||||
CachedPlanSource *plansource;
|
|
||||||
Query *query;
|
|
||||||
CachedPlan *cplan;
|
CachedPlan *cplan;
|
||||||
MemoryContext oldcontext;
|
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
|
* called immediately after creating the CachedPlanSource, we need not
|
||||||
* worry about the query being stale.
|
* 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);
|
plansources = SPI_plan_get_plan_sources(expr->plan);
|
||||||
if (list_length(plansources) != 1)
|
if (list_length(plansources) != 1)
|
||||||
return;
|
return false;
|
||||||
plansource = (CachedPlanSource *) linitial(plansources);
|
plansource = (CachedPlanSource *) linitial(plansources);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* 1. There must be one single querytree.
|
* 1. There must be one single querytree.
|
||||||
*/
|
*/
|
||||||
if (list_length(plansource->query_list) != 1)
|
if (list_length(plansource->query_list) != 1)
|
||||||
return;
|
return false;
|
||||||
query = (Query *) linitial(plansource->query_list);
|
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))
|
if (!IsA(query, Query))
|
||||||
return;
|
return false;
|
||||||
if (query->commandType != CMD_SELECT)
|
if (query->commandType != CMD_SELECT)
|
||||||
return;
|
return false;
|
||||||
if (query->rtable != NIL)
|
if (query->rtable != NIL)
|
||||||
return;
|
return false;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* 3. Can't have any subplans, aggregates, qual clauses either. (These
|
* 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->limitOffset ||
|
||||||
query->limitCount ||
|
query->limitCount ||
|
||||||
query->setOperations)
|
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)
|
if (list_length(query->targetList) != 1)
|
||||||
return;
|
return false;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* OK, we can treat it as a simple plan.
|
* 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));
|
return true;
|
||||||
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);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
104
src/pl/plpgsql/src/sql/plpgsql_simple.sql
Normal file
104
src/pl/plpgsql/src/sql/plpgsql_simple.sql
Normal file
@ -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();
|
Loading…
Reference in New Issue
Block a user