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

Commita160e92

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 parent2de059d commita160e92

File tree

4 files changed

+130
-14
lines changed

4 files changed

+130
-14
lines changed

‎doc/src/sgml/spi.sgml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,9 @@ int SPI_execute_extended(const char *<parameter>command</parameter>,
742742
<listitem>
743743
<para>
744744
<literal>true</literal> allows non-atomic execution of CALL and DO
745-
statements
745+
statements (but this field is ignored unless
746+
the <symbol>SPI_OPT_NONATOMIC</symbol> flag was passed
747+
to <function>SPI_connect_ext</function>)
746748
</para>
747749
</listitem>
748750
</varlistentry>
@@ -1883,7 +1885,9 @@ int SPI_execute_plan_extended(SPIPlanPtr <parameter>plan</parameter>,
18831885
<listitem>
18841886
<para>
18851887
<literal>true</literal> allows non-atomic execution of CALL and DO
1886-
statements
1888+
statements (but this field is ignored unless
1889+
the <symbol>SPI_OPT_NONATOMIC</symbol> flag was passed
1890+
to <function>SPI_connect_ext</function>)
18871891
</para>
18881892
</listitem>
18891893
</varlistentry>

‎src/backend/executor/spi.c

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2398,13 +2398,20 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
23982398
uint64my_processed=0;
23992399
SPITupleTable*my_tuptable=NULL;
24002400
intres=0;
2401+
boolallow_nonatomic;
24012402
boolpushed_active_snap= false;
24022403
ResourceOwnerplan_owner=options->owner;
24032404
SPICallbackArgspicallbackarg;
24042405
ErrorContextCallbackspierrcontext;
24052406
CachedPlan*cplan=NULL;
24062407
ListCell*lc1;
24072408

2409+
/*
2410+
* We allow nonatomic behavior only if options->allow_nonatomic is set
2411+
* *and* the SPI_OPT_NONATOMIC flag was given when connecting.
2412+
*/
2413+
allow_nonatomic=options->allow_nonatomic&& !_SPI_current->atomic;
2414+
24082415
/*
24092416
* Setup error traceback support for ereport()
24102417
*/
@@ -2424,12 +2431,17 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
24242431
* snapshot != InvalidSnapshot, read_only = false: use the given snapshot,
24252432
* modified by advancing its command ID before each querytree.
24262433
*
2427-
* snapshot == InvalidSnapshot, read_only = true: use the entry-time
2428-
* ActiveSnapshot, if any (if there isn't one, we run with no snapshot).
2434+
* snapshot == InvalidSnapshot, read_only = true: do nothing for queries
2435+
* that require no snapshot. For those that do, ensure that a Portal
2436+
* snapshot exists; then use that, or use the entry-time ActiveSnapshot if
2437+
* that exists and is different.
24292438
*
2430-
* snapshot == InvalidSnapshot, read_only = false: take a full new
2431-
* snapshot for each user command, and advance its command ID before each
2432-
* querytree within the command.
2439+
* snapshot == InvalidSnapshot, read_only = false: do nothing for queries
2440+
* that require no snapshot. For those that do, ensure that a Portal
2441+
* snapshot exists; then, in atomic execution (!allow_nonatomic) take a
2442+
* full new snapshot for each user command, and advance its command ID
2443+
* before each querytree within the command. In allow_nonatomic mode we
2444+
* just use the Portal snapshot unmodified.
24332445
*
24342446
* In the first two cases, we can just push the snap onto the stack once
24352447
* for the whole plan list.
@@ -2439,6 +2451,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
24392451
*/
24402452
if (snapshot!=InvalidSnapshot)
24412453
{
2454+
/* this intentionally tests the options field not the derived value */
24422455
Assert(!options->allow_nonatomic);
24432456
if (options->read_only)
24442457
{
@@ -2584,7 +2597,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
25842597
* Skip it when doing non-atomic execution, though (we rely
25852598
* entirely on the Portal snapshot in that case).
25862599
*/
2587-
if (!options->read_only&& !options->allow_nonatomic)
2600+
if (!options->read_only&& !allow_nonatomic)
25882601
{
25892602
if (pushed_active_snap)
25902603
PopActiveSnapshot();
@@ -2684,14 +2697,13 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
26842697
QueryCompletionqc;
26852698

26862699
/*
2687-
* If the SPI context is atomic, or we were not told to allow
2688-
* nonatomic operations, tell ProcessUtility this is an atomic
2689-
* execution context.
2700+
* If we're not allowing nonatomic operations, tell
2701+
* ProcessUtility this is an atomic execution context.
26902702
*/
2691-
if (_SPI_current->atomic|| !options->allow_nonatomic)
2692-
context=PROCESS_UTILITY_QUERY;
2693-
else
2703+
if (allow_nonatomic)
26942704
context=PROCESS_UTILITY_QUERY_NONATOMIC;
2705+
else
2706+
context=PROCESS_UTILITY_QUERY;
26952707

26962708
InitializeQueryCompletion(&qc);
26972709
ProcessUtility(stmt,

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

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

565565
(1 row)
566566

567+
-- Check that stable functions in CALL see the correct snapshot
568+
CREATE TABLE t_test (x int);
569+
INSERT INTO t_test VALUES (0);
570+
CREATE FUNCTION f_get_x () RETURNS int
571+
AS $$
572+
DECLARE l_result int;
573+
BEGIN
574+
SELECT x INTO l_result FROM t_test;
575+
RETURN l_result;
576+
END
577+
$$ LANGUAGE plpgsql STABLE;
578+
CREATE PROCEDURE f_print_x (x int)
579+
AS $$
580+
BEGIN
581+
RAISE NOTICE 'f_print_x(%)', x;
582+
END
583+
$$ LANGUAGE plpgsql;
584+
-- test in non-atomic context
585+
DO $$
586+
BEGIN
587+
UPDATE t_test SET x = x + 1;
588+
RAISE NOTICE 'f_get_x(%)', f_get_x();
589+
CALL f_print_x(f_get_x());
590+
UPDATE t_test SET x = x + 1;
591+
RAISE NOTICE 'f_get_x(%)', f_get_x();
592+
CALL f_print_x(f_get_x());
593+
ROLLBACK;
594+
END
595+
$$;
596+
NOTICE: f_get_x(1)
597+
NOTICE: f_print_x(1)
598+
NOTICE: f_get_x(2)
599+
NOTICE: f_print_x(2)
600+
-- test in atomic context
601+
BEGIN;
602+
DO $$
603+
BEGIN
604+
UPDATE t_test SET x = x + 1;
605+
RAISE NOTICE 'f_get_x(%)', f_get_x();
606+
CALL f_print_x(f_get_x());
607+
UPDATE t_test SET x = x + 1;
608+
RAISE NOTICE 'f_get_x(%)', f_get_x();
609+
CALL f_print_x(f_get_x());
610+
END
611+
$$;
612+
NOTICE: f_get_x(1)
613+
NOTICE: f_print_x(1)
614+
NOTICE: f_get_x(2)
615+
NOTICE: f_print_x(2)
616+
ROLLBACK;

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

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

523523
CALL outer_p(42);
524524
SELECT outer_f(42);
525+
526+
-- Check that stable functions in CALL see the correct snapshot
527+
528+
CREATETABLEt_test (xint);
529+
INSERT INTO t_testVALUES (0);
530+
531+
CREATEFUNCTIONf_get_x () RETURNSint
532+
AS $$
533+
DECLARE l_resultint;
534+
BEGIN
535+
SELECT x INTO l_resultFROM t_test;
536+
RETURN l_result;
537+
END
538+
$$ LANGUAGE plpgsql STABLE;
539+
540+
CREATE PROCEDURE f_print_x (xint)
541+
AS $$
542+
BEGIN
543+
RAISE NOTICE'f_print_x(%)', x;
544+
END
545+
$$ LANGUAGE plpgsql;
546+
547+
-- test in non-atomic context
548+
DO $$
549+
BEGIN
550+
UPDATE t_testSET x= x+1;
551+
RAISE NOTICE'f_get_x(%)', f_get_x();
552+
CALL f_print_x(f_get_x());
553+
UPDATE t_testSET x= x+1;
554+
RAISE NOTICE'f_get_x(%)', f_get_x();
555+
CALL f_print_x(f_get_x());
556+
ROLLBACK;
557+
END
558+
$$;
559+
560+
-- test in atomic context
561+
BEGIN;
562+
563+
DO $$
564+
BEGIN
565+
UPDATE t_testSET x= x+1;
566+
RAISE NOTICE'f_get_x(%)', f_get_x();
567+
CALL f_print_x(f_get_x());
568+
UPDATE t_testSET x= x+1;
569+
RAISE NOTICE'f_get_x(%)', f_get_x();
570+
CALL f_print_x(f_get_x());
571+
END
572+
$$;
573+
574+
ROLLBACK;

0 commit comments

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