Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd9809bf

Browse files
committed
Fix corner-case uninitialized-variable issues in plpgsql.
If an error was raised during our initial attempt to check whethera successfully-compiled expression is "simple", subsequent calls ofexec_stmt_execsql would suppose that stmt->mod_stmt was already computedwhen it had not been. This could lead to assertion failures in debugbuilds; in production builds the effect would typically be to act asif INTO STRICT had been specified even when it had not been. Of coursethat only matters if the subsequent attempt to execute the expressionsucceeds, so that the problem can only be reached by fixing a failurein some referenced, inline-able SQL function and then retrying thecalling plpgsql function in the same session.(There might be even-more-obscure ways to change the expression'sbehavior without changing the plpgsql function, but that one seemslike the only one people would be likely to hit in practice.)The most foolproof way to fix this would be to arrange forexec_prepare_plan to not set expr->plan until we've finished thesubsidiary simple-expression check. But it seems hard to do thatwithout creating reference-count leak issues. So settle for documentingthe hazard in a comment and fixing exec_stmt_execsql to test separatelyfor whether it's computed stmt->mod_stmt. (That adds a test-and-branchper execution, but hopefully that's negligible in context.) In v11 andup, also fix exec_stmt_call which had a variant of the same issue.Per bug #17113 from Alexander Lakhin. Back-patch to allsupported branches.Discussion:https://postgr.es/m/17113-077605ce00e0e7ec@postgresql.org
1 parent91d395f commitd9809bf

File tree

5 files changed

+84
-18
lines changed

5 files changed

+84
-18
lines changed

‎src/pl/plpgsql/src/expected/plpgsql_simple.out

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,26 @@ select simplecaller();
6666
555
6767
(1 row)
6868

69+
-- Check case where first attempt to determine if it's simple fails
70+
create function simplesql() returns int language sql
71+
as $$select 1 / 0$$;
72+
create or replace function simplecaller() returns int language plpgsql
73+
as $$
74+
declare x int;
75+
begin
76+
select simplesql() into x;
77+
return x;
78+
end$$;
79+
select simplecaller(); -- division by zero occurs during simple-expr check
80+
ERROR: division by zero
81+
CONTEXT: SQL function "simplesql" during inlining
82+
SQL statement "select simplesql()"
83+
PL/pgSQL function simplecaller() line 4 at SQL statement
84+
create or replace function simplesql() returns int language sql
85+
as $$select 2 + 2$$;
86+
select simplecaller();
87+
simplecaller
88+
--------------
89+
4
90+
(1 row)
91+

‎src/pl/plpgsql/src/pl_exec.c

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2162,22 +2162,20 @@ exec_stmt_call(PLpgSQL_execstate *estate, PLpgSQL_stmt_call *stmt)
21622162
* Make a plan if we don't have one already.
21632163
*/
21642164
if (expr->plan==NULL)
2165-
{
21662165
exec_prepare_plan(estate,expr,0);
21672166

2168-
/*
2169-
* A CALL or DO can never be a simple expression.
2170-
*/
2171-
Assert(!expr->expr_simple_expr);
2167+
/*
2168+
* A CALL or DO can never be a simple expression.
2169+
*/
2170+
Assert(!expr->expr_simple_expr);
21722171

2173-
/*
2174-
* Also construct a DTYPE_ROW datum representing the plpgsql variables
2175-
* associated with the procedure's output arguments. Then we can use
2176-
* exec_move_row() to do the assignments.
2177-
*/
2178-
if (stmt->is_call)
2179-
stmt->target=make_callstmt_target(estate,expr);
2180-
}
2172+
/*
2173+
* Also construct a DTYPE_ROW datum representing the plpgsql variables
2174+
* associated with the procedure's output arguments. Then we can use
2175+
* exec_move_row() to do the assignments.
2176+
*/
2177+
if (stmt->is_call&&stmt->target==NULL)
2178+
stmt->target=make_callstmt_target(estate,expr);
21812179

21822180
paramLI=setup_param_list(estate,expr);
21832181

@@ -4093,6 +4091,22 @@ exec_eval_cleanup(PLpgSQL_execstate *estate)
40934091

40944092
/* ----------
40954093
* Generate a prepared plan
4094+
*
4095+
* CAUTION: it is possible for this function to throw an error after it has
4096+
* built a SPIPlan and saved it in expr->plan. Therefore, be wary of doing
4097+
* additional things contingent on expr->plan being NULL. That is, given
4098+
* code like
4099+
*
4100+
*if (query->plan == NULL)
4101+
*{
4102+
*// okay to put setup code here
4103+
*exec_prepare_plan(estate, query, ...);
4104+
*// NOT okay to put more logic here
4105+
*}
4106+
*
4107+
* extra steps at the end are unsafe because they will not be executed when
4108+
* re-executing the calling statement, if exec_prepare_plan failed the first
4109+
* time. This is annoyingly error-prone, but the alternatives are worse.
40964110
* ----------
40974111
*/
40984112
staticvoid
@@ -4156,10 +4170,12 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
41564170
* whether the statement is INSERT/UPDATE/DELETE
41574171
*/
41584172
if (expr->plan==NULL)
4173+
exec_prepare_plan(estate,expr,CURSOR_OPT_PARALLEL_OK);
4174+
4175+
if (!stmt->mod_stmt_set)
41594176
{
41604177
ListCell*l;
41614178

4162-
exec_prepare_plan(estate,expr,CURSOR_OPT_PARALLEL_OK);
41634179
stmt->mod_stmt= false;
41644180
foreach(l,SPI_plan_get_plan_sources(expr->plan))
41654181
{
@@ -4179,6 +4195,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,
41794195
break;
41804196
}
41814197
}
4198+
stmt->mod_stmt_set= true;
41824199
}
41834200

41844201
/*
@@ -7846,6 +7863,10 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
78467863
* exec_simple_check_plan -Check if a plan is simple enough to
78477864
*be evaluated by ExecEvalExpr() instead
78487865
*of SPI.
7866+
*
7867+
* Note: the refcount manipulations in this function assume that expr->plan
7868+
* is a "saved" SPI plan. That's a bit annoying from the caller's standpoint,
7869+
* but it's otherwise difficult to avoid leaking the plan on failure.
78497870
* ----------
78507871
*/
78517872
staticvoid
@@ -7929,7 +7950,8 @@ exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
79297950
* OK, we can treat it as a simple plan.
79307951
*
79317952
* Get the generic plan for the query. If replanning is needed, do that
7932-
* work in the eval_mcontext.
7953+
* work in the eval_mcontext. (Note that replanning could throw an error,
7954+
* in which case the expr is left marked "not simple", which is fine.)
79337955
*/
79347956
oldcontext=MemoryContextSwitchTo(get_eval_mcontext(estate));
79357957
cplan=SPI_plan_get_cached_plan(expr->plan);

‎src/pl/plpgsql/src/pl_gram.y

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3043,7 +3043,7 @@ make_execsql_stmt(int firsttoken, int location)
30433043

30443044
check_sql_expr(expr->query, expr->parseMode, location);
30453045

3046-
execsql =palloc(sizeof(PLpgSQL_stmt_execsql));
3046+
execsql =palloc0(sizeof(PLpgSQL_stmt_execsql));
30473047
execsql->cmd_type = PLPGSQL_STMT_EXECSQL;
30483048
execsql->lineno = plpgsql_location_to_lineno(location);
30493049
execsql->stmtid = ++plpgsql_curr_compile->nstatements;

‎src/pl/plpgsql/src/plpgsql.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -893,8 +893,8 @@ typedef struct PLpgSQL_stmt_execsql
893893
intlineno;
894894
unsignedintstmtid;
895895
PLpgSQL_expr*sqlstmt;
896-
boolmod_stmt;/* is the stmt INSERT/UPDATE/DELETE? Note:
897-
* mod_stmtisset when we plan the query */
896+
boolmod_stmt;/* is the stmt INSERT/UPDATE/DELETE?*/
897+
boolmod_stmt_set;/*ismod_stmt valid yet? */
898898
boolinto;/* INTO supplied? */
899899
boolstrict;/* INTO STRICT flag */
900900
PLpgSQL_variable*target;/* INTO target (record or row) */

‎src/pl/plpgsql/src/sql/plpgsql_simple.sql

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,24 @@ select simplecaller();
5959
\c-
6060

6161
select simplecaller();
62+
63+
64+
-- Check case where first attempt to determine if it's simple fails
65+
66+
createfunctionsimplesql() returnsint language sql
67+
as $$select1/0$$;
68+
69+
create or replacefunctionsimplecaller() returnsint language plpgsql
70+
as $$
71+
declare xint;
72+
begin
73+
select simplesql() into x;
74+
return x;
75+
end$$;
76+
77+
select simplecaller();-- division by zero occurs during simple-expr check
78+
79+
create or replacefunctionsimplesql() returnsint language sql
80+
as $$select2+2$$;
81+
82+
select simplecaller();

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp