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

Commit2dc1dea

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 parentd92573a commit2dc1dea

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
@@ -752,7 +752,9 @@ int SPI_execute_extended(const char *<parameter>command</parameter>,
752752
<listitem>
753753
<para>
754754
<literal>true</literal> allows non-atomic execution of CALL and DO
755-
statements
755+
statements (but this field is ignored unless
756+
the <symbol>SPI_OPT_NONATOMIC</symbol> flag was passed
757+
to <function>SPI_connect_ext</function>)
756758
</para>
757759
</listitem>
758760
</varlistentry>
@@ -1893,7 +1895,9 @@ int SPI_execute_plan_extended(SPIPlanPtr <parameter>plan</parameter>,
18931895
<listitem>
18941896
<para>
18951897
<literal>true</literal> allows non-atomic execution of CALL and DO
1896-
statements
1898+
statements (but this field is ignored unless
1899+
the <symbol>SPI_OPT_NONATOMIC</symbol> flag was passed
1900+
to <function>SPI_connect_ext</function>)
18971901
</para>
18981902
</listitem>
18991903
</varlistentry>

‎src/backend/executor/spi.c

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

2410+
/*
2411+
* We allow nonatomic behavior only if options->allow_nonatomic is set
2412+
* *and* the SPI_OPT_NONATOMIC flag was given when connecting.
2413+
*/
2414+
allow_nonatomic=options->allow_nonatomic&& !_SPI_current->atomic;
2415+
24092416
/*
24102417
* Setup error traceback support for ereport()
24112418
*/
@@ -2425,12 +2432,17 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
24252432
* snapshot != InvalidSnapshot, read_only = false: use the given snapshot,
24262433
* modified by advancing its command ID before each querytree.
24272434
*
2428-
* snapshot == InvalidSnapshot, read_only = true: use the entry-time
2429-
* ActiveSnapshot, if any (if there isn't one, we run with no snapshot).
2435+
* snapshot == InvalidSnapshot, read_only = true: do nothing for queries
2436+
* that require no snapshot. For those that do, ensure that a Portal
2437+
* snapshot exists; then use that, or use the entry-time ActiveSnapshot if
2438+
* that exists and is different.
24302439
*
2431-
* snapshot == InvalidSnapshot, read_only = false: take a full new
2432-
* snapshot for each user command, and advance its command ID before each
2433-
* querytree within the command.
2440+
* snapshot == InvalidSnapshot, read_only = false: do nothing for queries
2441+
* that require no snapshot. For those that do, ensure that a Portal
2442+
* snapshot exists; then, in atomic execution (!allow_nonatomic) take a
2443+
* full new snapshot for each user command, and advance its command ID
2444+
* before each querytree within the command. In allow_nonatomic mode we
2445+
* just use the Portal snapshot unmodified.
24342446
*
24352447
* In the first two cases, we can just push the snap onto the stack once
24362448
* for the whole plan list.
@@ -2440,6 +2452,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
24402452
*/
24412453
if (snapshot!=InvalidSnapshot)
24422454
{
2455+
/* this intentionally tests the options field not the derived value */
24432456
Assert(!options->allow_nonatomic);
24442457
if (options->read_only)
24452458
{
@@ -2585,7 +2598,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
25852598
* Skip it when doing non-atomic execution, though (we rely
25862599
* entirely on the Portal snapshot in that case).
25872600
*/
2588-
if (!options->read_only&& !options->allow_nonatomic)
2601+
if (!options->read_only&& !allow_nonatomic)
25892602
{
25902603
if (pushed_active_snap)
25912604
PopActiveSnapshot();
@@ -2685,14 +2698,13 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
26852698
QueryCompletionqc;
26862699

26872700
/*
2688-
* If the SPI context is atomic, or we were not told to allow
2689-
* nonatomic operations, tell ProcessUtility this is an atomic
2690-
* execution context.
2701+
* If we're not allowing nonatomic operations, tell
2702+
* ProcessUtility this is an atomic execution context.
26912703
*/
2692-
if (_SPI_current->atomic|| !options->allow_nonatomic)
2693-
context=PROCESS_UTILITY_QUERY;
2694-
else
2704+
if (allow_nonatomic)
26952705
context=PROCESS_UTILITY_QUERY_NONATOMIC;
2706+
else
2707+
context=PROCESS_UTILITY_QUERY;
26962708

26972709
InitializeQueryCompletion(&qc);
26982710
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