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

Commit1d4ea13

Browse files
committed
Fix behavior of stable functions called from a CALL's argument list.
If the CALL is within an atomic context (e.g. there's an outertransaction block), _SPI_execute_plan should acquire a fresh snapshotto execute any such functions with. We failed to do that and insteadpassed them the Portal snapshot, which had been acquired at the startof the current SQL command. This'd lead to seeing stale values ofrows modified since the start of the command.This is arguably a bug in84f5c29: I failed to see that "are we innon-atomic mode" needs to be defined the same way as it is furtherdown in _SPI_execute_plan, i.e. check !_SPI_current->atomic not justoptions->allow_nonatomic. Alternatively the blame could be laid onplpgsql, which is unconditionally passing allow_nonatomic = truefor CALL/DO even when it knows it's in an atomic context. However,fixing it in spi.c seems like a better idea since that will also fixthe problem for any extensions that may have copied plpgsql's codingpattern.While here, update an obsolete comment about _SPI_execute_plan'ssnapshot management.Per report from Victor Yegorov. Back-patch to all supported versions.Discussion:https://postgr.es/m/CAGnEboiRe+fG2QxuBO2390F7P8e2MQ6UyBjZSL_w1Cej+E4=Vw@mail.gmail.com
1 parent2b461ef commit1d4ea13

File tree

3 files changed

+124
-13
lines changed

3 files changed

+124
-13
lines changed

‎src/backend/executor/spi.c

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2204,12 +2204,18 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
22042204
uint64my_processed=0;
22052205
SPITupleTable*my_tuptable=NULL;
22062206
intres=0;
2207-
boolallow_nonatomic=plan->no_snapshots;/* legacy API name */
2207+
boolallow_nonatomic;
22082208
boolpushed_active_snap= false;
22092209
ErrorContextCallbackspierrcontext;
22102210
CachedPlan*cplan=NULL;
22112211
ListCell*lc1;
22122212

2213+
/*
2214+
* We allow nonatomic behavior only if plan->no_snapshots is set
2215+
* *and* the SPI_OPT_NONATOMIC flag was given when connecting.
2216+
*/
2217+
allow_nonatomic=plan->no_snapshots&& !_SPI_current->atomic;
2218+
22132219
/*
22142220
* Setup error traceback support for ereport()
22152221
*/
@@ -2227,12 +2233,17 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
22272233
* snapshot != InvalidSnapshot, read_only = false: use the given snapshot,
22282234
* modified by advancing its command ID before each querytree.
22292235
*
2230-
* snapshot == InvalidSnapshot, read_only = true: use the entry-time
2231-
* ActiveSnapshot, if any (if there isn't one, we run with no snapshot).
2236+
* snapshot == InvalidSnapshot, read_only = true: do nothing for queries
2237+
* that require no snapshot. For those that do, ensure that a Portal
2238+
* snapshot exists; then use that, or use the entry-time ActiveSnapshot if
2239+
* that exists and is different.
22322240
*
2233-
* snapshot == InvalidSnapshot, read_only = false: take a full new
2234-
* snapshot for each user command, and advance its command ID before each
2235-
* querytree within the command.
2241+
* snapshot == InvalidSnapshot, read_only = false: do nothing for queries
2242+
* that require no snapshot. For those that do, ensure that a Portal
2243+
* snapshot exists; then, in atomic execution (!allow_nonatomic) take a
2244+
* full new snapshot for each user command, and advance its command ID
2245+
* before each querytree within the command. In allow_nonatomic mode we
2246+
* just use the Portal snapshot unmodified.
22362247
*
22372248
* In the first two cases, we can just push the snap onto the stack once
22382249
* for the whole plan list.
@@ -2242,7 +2253,8 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
22422253
*/
22432254
if (snapshot!=InvalidSnapshot)
22442255
{
2245-
Assert(!allow_nonatomic);
2256+
/* this intentionally tests the plan field not the derived value */
2257+
Assert(!plan->no_snapshots);
22462258
if (read_only)
22472259
{
22482260
PushActiveSnapshot(snapshot);
@@ -2427,14 +2439,13 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
24272439
QueryCompletionqc;
24282440

24292441
/*
2430-
* If the SPI context is atomic, or we were not told to allow
2431-
* nonatomic operations, tell ProcessUtility this is an atomic
2432-
* execution context.
2442+
* If we're not allowing nonatomic operations, tell
2443+
* ProcessUtility this is an atomic execution context.
24332444
*/
2434-
if (_SPI_current->atomic|| !allow_nonatomic)
2435-
context=PROCESS_UTILITY_QUERY;
2436-
else
2445+
if (allow_nonatomic)
24372446
context=PROCESS_UTILITY_QUERY_NONATOMIC;
2447+
else
2448+
context=PROCESS_UTILITY_QUERY;
24382449

24392450
InitializeQueryCompletion(&qc);
24402451
ProcessUtility(stmt,

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,3 +457,53 @@ NOTICE: inner_p(44)
457457

458458
(1 row)
459459

460+
-- Check that stable functions in CALL see the correct snapshot
461+
CREATE TABLE t_test (x int);
462+
INSERT INTO t_test VALUES (0);
463+
CREATE FUNCTION f_get_x () RETURNS int
464+
AS $$
465+
DECLARE l_result int;
466+
BEGIN
467+
SELECT x INTO l_result FROM t_test;
468+
RETURN l_result;
469+
END
470+
$$ LANGUAGE plpgsql STABLE;
471+
CREATE PROCEDURE f_print_x (x int)
472+
AS $$
473+
BEGIN
474+
RAISE NOTICE 'f_print_x(%)', x;
475+
END
476+
$$ LANGUAGE plpgsql;
477+
-- test in non-atomic context
478+
DO $$
479+
BEGIN
480+
UPDATE t_test SET x = x + 1;
481+
RAISE NOTICE 'f_get_x(%)', f_get_x();
482+
CALL f_print_x(f_get_x());
483+
UPDATE t_test SET x = x + 1;
484+
RAISE NOTICE 'f_get_x(%)', f_get_x();
485+
CALL f_print_x(f_get_x());
486+
ROLLBACK;
487+
END
488+
$$;
489+
NOTICE: f_get_x(1)
490+
NOTICE: f_print_x(1)
491+
NOTICE: f_get_x(2)
492+
NOTICE: f_print_x(2)
493+
-- test in atomic context
494+
BEGIN;
495+
DO $$
496+
BEGIN
497+
UPDATE t_test SET x = x + 1;
498+
RAISE NOTICE 'f_get_x(%)', f_get_x();
499+
CALL f_print_x(f_get_x());
500+
UPDATE t_test SET x = x + 1;
501+
RAISE NOTICE 'f_get_x(%)', f_get_x();
502+
CALL f_print_x(f_get_x());
503+
END
504+
$$;
505+
NOTICE: f_get_x(1)
506+
NOTICE: f_print_x(1)
507+
NOTICE: f_get_x(2)
508+
NOTICE: f_print_x(2)
509+
ROLLBACK;

‎src/pl/plpgsql/src/sql/plpgsql_call.sql

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,3 +425,53 @@ CREATE FUNCTION f(int) RETURNS int AS $$ SELECT $1 + 2 $$ LANGUAGE sql;
425425

426426
CALL outer_p(42);
427427
SELECT outer_f(42);
428+
429+
-- Check that stable functions in CALL see the correct snapshot
430+
431+
CREATETABLEt_test (xint);
432+
INSERT INTO t_testVALUES (0);
433+
434+
CREATEFUNCTIONf_get_x () RETURNSint
435+
AS $$
436+
DECLARE l_resultint;
437+
BEGIN
438+
SELECT x INTO l_resultFROM t_test;
439+
RETURN l_result;
440+
END
441+
$$ LANGUAGE plpgsql STABLE;
442+
443+
CREATE PROCEDURE f_print_x (xint)
444+
AS $$
445+
BEGIN
446+
RAISE NOTICE'f_print_x(%)', x;
447+
END
448+
$$ LANGUAGE plpgsql;
449+
450+
-- test in non-atomic context
451+
DO $$
452+
BEGIN
453+
UPDATE t_testSET x= x+1;
454+
RAISE NOTICE'f_get_x(%)', f_get_x();
455+
CALL f_print_x(f_get_x());
456+
UPDATE t_testSET x= x+1;
457+
RAISE NOTICE'f_get_x(%)', f_get_x();
458+
CALL f_print_x(f_get_x());
459+
ROLLBACK;
460+
END
461+
$$;
462+
463+
-- test in atomic context
464+
BEGIN;
465+
466+
DO $$
467+
BEGIN
468+
UPDATE t_testSET x= x+1;
469+
RAISE NOTICE'f_get_x(%)', f_get_x();
470+
CALL f_print_x(f_get_x());
471+
UPDATE t_testSET x= x+1;
472+
RAISE NOTICE'f_get_x(%)', f_get_x();
473+
CALL f_print_x(f_get_x());
474+
END
475+
$$;
476+
477+
ROLLBACK;

0 commit comments

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