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

Commit84f5c29

Browse files
committed
Restore the portal-level snapshot after procedure COMMIT/ROLLBACK.
COMMIT/ROLLBACK necessarily destroys all snapshots within the session.The original implementation of intra-procedure transactions justcavalierly did that, ignoring the fact that this left us executing ina rather different environment than normal. In particular, it turnsout that handling of toasted datums depends rather critically on therebeing an outer ActiveSnapshot: otherwise, when SPI or the coreexecutor pop whatever snapshot they used and return, it's unsafe todereference any toasted datums that may appear in the query result.It's possible to demonstrate "no known snapshots" and "missing chunknumber N for toast value" errors as a result of this oversight.Historically this outer snapshot has been held by the Portal code,and that seems like a good plan to preserve. So add infrastructureto pquery.c to allow re-establishing the Portal-owned snapshot if it'snot there anymore, and add enough bookkeeping support that we can tellwhether it is or not.We can't, however, just re-establish the Portal snapshot as part ofCOMMIT/ROLLBACK. As in normal transaction start, acquiring the firstsnapshot should wait until after SET and LOCK commands. Hence, teachspi.c about doing this at the right time. (Note that this patchdoesn't fix the problem for any PLs that try to run intra-proceduretransactions without using SPI to execute SQL commands.)This makes SPI's no_snapshots parameter rather a misnomer, so in HEAD,rename that to allow_nonatomic.replication/logical/worker.c also needs some fixes, because it wasn'tcareful to hold a snapshot open around AFTER trigger execution.That code doesn't use a Portal, which I suspect someday we're gonnahave to fix. But for now, just rearrange the order of operations.This includes back-patching the recent addition of finish_estate()to centralize the cleanup logic there.This also back-patches commit2ecfeda into v13, to improve thetest coverage for worker.c (it was that test that exposed thatworker.c's snapshot management is wrong).Per bug #15990 from Andreas Wicht. Back-patch to v11 whereintra-procedure COMMIT was added.Discussion:https://postgr.es/m/15990-eee2ac466b11293d@postgresql.org
1 parent124966c commit84f5c29

File tree

12 files changed

+312
-107
lines changed

12 files changed

+312
-107
lines changed

‎doc/src/sgml/spi.sgml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -730,11 +730,11 @@ int SPI_execute_extended(const char *<parameter>command</parameter>,
730730
</varlistentry>
731731

732732
<varlistentry>
733-
<term><literal>bool <parameter>no_snapshots</parameter></literal></term>
733+
<term><literal>bool <parameter>allow_nonatomic</parameter></literal></term>
734734
<listitem>
735735
<para>
736-
<literal>true</literal>prevents SPI from managing snapshots for
737-
execution of the query; use with extreme caution
736+
<literal>true</literal>allows non-atomic execution of CALL and DO
737+
statements
738738
</para>
739739
</listitem>
740740
</varlistentry>
@@ -1860,11 +1860,11 @@ int SPI_execute_plan_extended(SPIPlanPtr <parameter>plan</parameter>,
18601860
</varlistentry>
18611861

18621862
<varlistentry>
1863-
<term><literal>bool <parameter>no_snapshots</parameter></literal></term>
1863+
<term><literal>bool <parameter>allow_nonatomic</parameter></literal></term>
18641864
<listitem>
18651865
<para>
1866-
<literal>true</literal>prevents SPI from managing snapshots for
1867-
execution of the query; use with extreme caution
1866+
<literal>true</literal>allows non-atomic execution of CALL and DO
1867+
statements
18681868
</para>
18691869
</listitem>
18701870
</varlistentry>

‎src/backend/commands/functioncmds.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@
6464
#include"parser/parse_func.h"
6565
#include"parser/parse_type.h"
6666
#include"pgstat.h"
67+
#include"tcop/pquery.h"
6768
#include"tcop/utility.h"
6869
#include"utils/acl.h"
6970
#include"utils/builtins.h"
@@ -2319,6 +2320,20 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver
23192320
if (fcinfo->isnull)
23202321
elog(ERROR,"procedure returned null record");
23212322

2323+
/*
2324+
* Ensure there's an active snapshot whilst we execute whatever's
2325+
* involved here. Note that this is *not* sufficient to make the
2326+
* world safe for TOAST pointers to be included in the returned data:
2327+
* the referenced data could have gone away while we didn't hold a
2328+
* snapshot. Hence, it's incumbent on PLs that can do COMMIT/ROLLBACK
2329+
* to not return TOAST pointers, unless those pointers were fetched
2330+
* after the last COMMIT/ROLLBACK in the procedure.
2331+
*
2332+
* XXX that is a really nasty, hard-to-test requirement. Is there a
2333+
* way to remove it?
2334+
*/
2335+
EnsurePortalSnapshotExists();
2336+
23222337
td=DatumGetHeapTupleHeader(retval);
23232338
tupType=HeapTupleHeaderGetTypeId(td);
23242339
tupTypmod=HeapTupleHeaderGetTypMod(td);

‎src/backend/executor/spi.c

Lines changed: 53 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ static void _SPI_prepare_oneshot_plan(const char *src, SPIPlanPtr plan);
6666

6767
staticint_SPI_execute_plan(SPIPlanPtrplan,ParamListInfoparamLI,
6868
Snapshotsnapshot,Snapshotcrosscheck_snapshot,
69-
boolread_only,boolno_snapshots,
69+
boolread_only,boolallow_nonatomic,
7070
boolfire_triggers,uint64tcount,
7171
DestReceiver*caller_dest,
7272
ResourceOwnerplan_owner);
@@ -260,12 +260,8 @@ _SPI_commit(bool chain)
260260
/* Start the actual commit */
261261
_SPI_current->internal_xact= true;
262262

263-
/*
264-
* Before committing, pop all active snapshots to avoid error about
265-
* "snapshot %p still active".
266-
*/
267-
while (ActiveSnapshotSet())
268-
PopActiveSnapshot();
263+
/* Release snapshots associated with portals */
264+
ForgetPortalSnapshots();
269265

270266
if (chain)
271267
SaveTransactionCharacteristics();
@@ -322,6 +318,9 @@ _SPI_rollback(bool chain)
322318
/* Start the actual rollback */
323319
_SPI_current->internal_xact= true;
324320

321+
/* Release snapshots associated with portals */
322+
ForgetPortalSnapshots();
323+
325324
if (chain)
326325
SaveTransactionCharacteristics();
327326

@@ -567,7 +566,7 @@ SPI_execute_extended(const char *src,
567566

568567
res=_SPI_execute_plan(&plan,options->params,
569568
InvalidSnapshot,InvalidSnapshot,
570-
options->read_only,options->no_snapshots,
569+
options->read_only,options->allow_nonatomic,
571570
true,options->tcount,
572571
options->dest,options->owner);
573572

@@ -627,7 +626,7 @@ SPI_execute_plan_extended(SPIPlanPtr plan,
627626

628627
res=_SPI_execute_plan(plan,options->params,
629628
InvalidSnapshot,InvalidSnapshot,
630-
options->read_only,options->no_snapshots,
629+
options->read_only,options->allow_nonatomic,
631630
true,options->tcount,
632631
options->dest,options->owner);
633632

@@ -2264,7 +2263,7 @@ _SPI_prepare_oneshot_plan(const char *src, SPIPlanPtr plan)
22642263
*behavior of taking a new snapshot for each query.
22652264
* crosscheck_snapshot: for RI use, all others pass InvalidSnapshot
22662265
* read_only: true for read-only execution (no CommandCounterIncrement)
2267-
*no_snapshots: true toskip snapshot management
2266+
*allow_nonatomic: true toallow nonatomic CALL/DO execution
22682267
* fire_triggers: true to fire AFTER triggers at end of query (normal case);
22692268
*false means any AFTER triggers are postponed to end of outer query
22702269
* tcount: execution tuple-count limit, or 0 for none
@@ -2275,7 +2274,7 @@ _SPI_prepare_oneshot_plan(const char *src, SPIPlanPtr plan)
22752274
staticint
22762275
_SPI_execute_plan(SPIPlanPtrplan,ParamListInfoparamLI,
22772276
Snapshotsnapshot,Snapshotcrosscheck_snapshot,
2278-
boolread_only,boolno_snapshots,
2277+
boolread_only,boolallow_nonatomic,
22792278
boolfire_triggers,uint64tcount,
22802279
DestReceiver*caller_dest,ResourceOwnerplan_owner)
22812280
{
@@ -2318,11 +2317,12 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
23182317
* In the first two cases, we can just push the snap onto the stack once
23192318
* for the whole plan list.
23202319
*
2321-
*But if no_snapshots is true, then don't manage snapshots at all here.
2322-
*The caller must then take care of that.
2320+
*Note that snapshot != InvalidSnapshot implies an atomic execution
2321+
*context.
23232322
*/
2324-
if (snapshot!=InvalidSnapshot&& !no_snapshots)
2323+
if (snapshot!=InvalidSnapshot)
23252324
{
2325+
Assert(!allow_nonatomic);
23262326
if (read_only)
23272327
{
23282328
PushActiveSnapshot(snapshot);
@@ -2408,15 +2408,39 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
24082408
stmt_list=cplan->stmt_list;
24092409

24102410
/*
2411-
*In the default non-read-only case, get a new snapshot, replacing
2412-
*any that we pushed in a previous cycle.
2411+
*If we weren't given a specific snapshot to use, and the statement
2412+
*list requires a snapshot, set that up.
24132413
*/
2414-
if (snapshot==InvalidSnapshot&& !read_only&& !no_snapshots)
2414+
if (snapshot==InvalidSnapshot&&
2415+
(list_length(stmt_list)>1||
2416+
(list_length(stmt_list)==1&&
2417+
PlannedStmtRequiresSnapshot(linitial_node(PlannedStmt,
2418+
stmt_list)))))
24152419
{
2416-
if (pushed_active_snap)
2417-
PopActiveSnapshot();
2418-
PushActiveSnapshot(GetTransactionSnapshot());
2419-
pushed_active_snap= true;
2420+
/*
2421+
* First, ensure there's a Portal-level snapshot. This back-fills
2422+
* the snapshot stack in case the previous operation was a COMMIT
2423+
* or ROLLBACK inside a procedure or DO block. (We can't put back
2424+
* the Portal snapshot any sooner, or we'd break cases like doing
2425+
* SET or LOCK just after COMMIT.) It's enough to check once per
2426+
* statement list, since COMMIT/ROLLBACK/CALL/DO can't appear
2427+
* within a multi-statement list.
2428+
*/
2429+
EnsurePortalSnapshotExists();
2430+
2431+
/*
2432+
* In the default non-read-only case, get a new per-statement-list
2433+
* snapshot, replacing any that we pushed in a previous cycle.
2434+
* Skip it when doing non-atomic execution, though (we rely
2435+
* entirely on the Portal snapshot in that case).
2436+
*/
2437+
if (!read_only&& !allow_nonatomic)
2438+
{
2439+
if (pushed_active_snap)
2440+
PopActiveSnapshot();
2441+
PushActiveSnapshot(GetTransactionSnapshot());
2442+
pushed_active_snap= true;
2443+
}
24202444
}
24212445

24222446
foreach(lc2,stmt_list)
@@ -2434,6 +2458,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
24342458
_SPI_current->processed=0;
24352459
_SPI_current->tuptable=NULL;
24362460

2461+
/* Check for unsupported cases. */
24372462
if (stmt->utilityStmt)
24382463
{
24392464
if (IsA(stmt->utilityStmt,CopyStmt))
@@ -2462,9 +2487,10 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
24622487

24632488
/*
24642489
* If not read-only mode, advance the command counter before each
2465-
* command and update the snapshot.
2490+
* command and update the snapshot. (But skip it if the snapshot
2491+
* isn't under our control.)
24662492
*/
2467-
if (!read_only&&!no_snapshots)
2493+
if (!read_only&&pushed_active_snap)
24682494
{
24692495
CommandCounterIncrement();
24702496
UpdateActiveSnapshotCommandId();
@@ -2507,13 +2533,11 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
25072533
QueryCompletionqc;
25082534

25092535
/*
2510-
* If the SPI context is atomic, or we are asked to manage
2511-
* snapshots, then we are in an atomic execution context.
2512-
* Conversely, to propagate a nonatomic execution context, the
2513-
* caller must be in a nonatomic SPI context and manage
2514-
* snapshots itself.
2536+
* If the SPI context is atomic, or we were not told to allow
2537+
* nonatomic operations, tell ProcessUtility this is an atomic
2538+
* execution context.
25152539
*/
2516-
if (_SPI_current->atomic|| !no_snapshots)
2540+
if (_SPI_current->atomic|| !allow_nonatomic)
25172541
context=PROCESS_UTILITY_QUERY;
25182542
else
25192543
context=PROCESS_UTILITY_QUERY_NONATOMIC;

‎src/backend/replication/logical/worker.c

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,13 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel,
349349
EState*estate;
350350
RangeTblEntry*rte;
351351

352+
/*
353+
* Input functions may need an active snapshot, as may AFTER triggers
354+
* invoked during finish_estate. For safety, ensure an active snapshot
355+
* exists throughout all our usage of the executor.
356+
*/
357+
PushActiveSnapshot(GetTransactionSnapshot());
358+
352359
estate=CreateExecutorState();
353360

354361
rte=makeNode(RangeTblEntry);
@@ -400,6 +407,7 @@ finish_estate(EState *estate)
400407
/* Cleanup. */
401408
ExecResetTupleTable(estate->es_tupleTable, false);
402409
FreeExecutorState(estate);
410+
PopActiveSnapshot();
403411
}
404412

405413
/*
@@ -1212,9 +1220,6 @@ apply_handle_insert(StringInfo s)
12121220
RelationGetDescr(rel->localrel),
12131221
&TTSOpsVirtual);
12141222

1215-
/* Input functions may need an active snapshot, so get one */
1216-
PushActiveSnapshot(GetTransactionSnapshot());
1217-
12181223
/* Process and store remote tuple in the slot */
12191224
oldctx=MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
12201225
slot_store_data(remoteslot,rel,&newtup);
@@ -1229,8 +1234,6 @@ apply_handle_insert(StringInfo s)
12291234
apply_handle_insert_internal(resultRelInfo,estate,
12301235
remoteslot);
12311236

1232-
PopActiveSnapshot();
1233-
12341237
finish_estate(estate);
12351238

12361239
logicalrep_rel_close(rel,NoLock);
@@ -1358,8 +1361,6 @@ apply_handle_update(StringInfo s)
13581361
/* Also populate extraUpdatedCols, in case we have generated columns */
13591362
fill_extraUpdatedCols(target_rte,rel->localrel);
13601363

1361-
PushActiveSnapshot(GetTransactionSnapshot());
1362-
13631364
/* Build the search tuple. */
13641365
oldctx=MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
13651366
slot_store_data(remoteslot,rel,
@@ -1374,8 +1375,6 @@ apply_handle_update(StringInfo s)
13741375
apply_handle_update_internal(resultRelInfo,estate,
13751376
remoteslot,&newtup,rel);
13761377

1377-
PopActiveSnapshot();
1378-
13791378
finish_estate(estate);
13801379

13811380
logicalrep_rel_close(rel,NoLock);
@@ -1482,8 +1481,6 @@ apply_handle_delete(StringInfo s)
14821481
RelationGetDescr(rel->localrel),
14831482
&TTSOpsVirtual);
14841483

1485-
PushActiveSnapshot(GetTransactionSnapshot());
1486-
14871484
/* Build the search tuple. */
14881485
oldctx=MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
14891486
slot_store_data(remoteslot,rel,&oldtup);
@@ -1497,8 +1494,6 @@ apply_handle_delete(StringInfo s)
14971494
apply_handle_delete_internal(resultRelInfo,estate,
14981495
remoteslot,&rel->remoterel);
14991496

1500-
PopActiveSnapshot();
1501-
15021497
finish_estate(estate);
15031498

15041499
logicalrep_rel_close(rel,NoLock);
@@ -1818,7 +1813,7 @@ apply_handle_truncate(StringInfo s)
18181813
List*relids=NIL;
18191814
List*relids_logged=NIL;
18201815
ListCell*lc;
1821-
LOCKMODElockmode=AccessExclusiveLock;
1816+
LOCKMODElockmode=AccessExclusiveLock;
18221817

18231818
if (handle_streamed_transaction(LOGICAL_REP_MSG_TRUNCATE,s))
18241819
return;

0 commit comments

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