database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-25 01:02:05 +03:00
Code Activity

Restore the portal-level snapshot after procedure COMMIT/ROLLBACK.

Browse Source 
COMMIT/ROLLBACK necessarily destroys all snapshots within the session.
The original implementation of intra-procedure transactions just
cavalierly did that, ignoring the fact that this left us executing in
a rather different environment than normal.  In particular, it turns
out that handling of toasted datums depends rather critically on there
being an outer ActiveSnapshot: otherwise, when SPI or the core
executor pop whatever snapshot they used and return, it's unsafe to
dereference any toasted datums that may appear in the query result.
It's possible to demonstrate "no known snapshots" and "missing chunk
number 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 infrastructure
to pquery.c to allow re-establishing the Portal-owned snapshot if it's
not there anymore, and add enough bookkeeping support that we can tell
whether it is or not.

We can't, however, just re-establish the Portal snapshot as part of
COMMIT/ROLLBACK.  As in normal transaction start, acquiring the first
snapshot should wait until after SET and LOCK commands.  Hence, teach
spi.c about doing this at the right time.  (Note that this patch
doesn't fix the problem for any PLs that try to run intra-procedure
transactions 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't
careful to hold a snapshot open around AFTER trigger execution.
That code doesn't use a Portal, which I suspect someday we're gonna
have 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 commit 2ecfeda3e into v13, to improve the
test coverage for worker.c (it was that test that exposed that
worker.c's snapshot management is wrong).

Per bug #15990 from Andreas Wicht.  Back-patch to v11 where
intra-procedure COMMIT was added.

Discussion: https://postgr.es/m/15990-eee2ac466b11293d@postgresql.org
This commit is contained in:
Tom Lane
2021-05-21 14:03:53 -04:00
parent 71787b23e3
commit ef94805096
11 changed files with 323 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
src/backend/replication/logical/worker.c
View File

@ -194,6 +194,13 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel)
ResultRelInfo *resultRelInfo;
RangeTblEntry *rte;
/*
* Input functions may need an active snapshot, as may AFTER triggers
* invoked during finish_estate. For safety, ensure an active snapshot
* exists throughout all our usage of the executor.
*/
PushActiveSnapshot(GetTransactionSnapshot());
estate = CreateExecutorState();
rte = makeNode(RangeTblEntry);
@ -221,6 +228,22 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel)
return estate;
}
/*
* Finish any operations related to the executor state created by
* create_estate_for_relation().
*/
static void
finish_estate(EState *estate)
{
/* Handle any queued AFTER triggers. */
AfterTriggerEndQuery(estate);
/* Cleanup. */
ExecResetTupleTable(estate->es_tupleTable, false);
FreeExecutorState(estate);
PopActiveSnapshot();
}
/*
* Executes default values for columns for which we can't map to remote
* relation columns.
@ -627,9 +650,6 @@ apply_handle_insert(StringInfo s)
remoteslot = ExecInitExtraTupleSlot(estate,
RelationGetDescr(rel->localrel));
/* Input functions may need an active snapshot, so get one */
PushActiveSnapshot(GetTransactionSnapshot());
/* Process and store remote tuple in the slot */
oldctx = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
slot_store_cstrings(remoteslot, rel, newtup.values);
@ -643,13 +663,8 @@ apply_handle_insert(StringInfo s)
/* Cleanup. */
ExecCloseIndices(estate->es_result_relation_info);
PopActiveSnapshot();
/* Handle queued AFTER triggers. */
AfterTriggerEndQuery(estate);
ExecResetTupleTable(estate->es_tupleTable, false);
FreeExecutorState(estate);
finish_estate(estate);
logicalrep_rel_close(rel, NoLock);
@ -760,7 +775,6 @@ apply_handle_update(StringInfo s)
}
}
PushActiveSnapshot(GetTransactionSnapshot());
ExecOpenIndices(estate->es_result_relation_info, false);
/* Build the search tuple. */
@ -819,15 +833,10 @@ apply_handle_update(StringInfo s)
}
/* Cleanup. */
ExecCloseIndices(estate->es_result_relation_info);
PopActiveSnapshot();
/* Handle queued AFTER triggers. */
AfterTriggerEndQuery(estate);
EvalPlanQualEnd(&epqstate);
ExecResetTupleTable(estate->es_tupleTable, false);
FreeExecutorState(estate);
ExecCloseIndices(estate->es_result_relation_info);
finish_estate(estate);
logicalrep_rel_close(rel, NoLock);
@ -878,7 +887,6 @@ apply_handle_delete(StringInfo s)
RelationGetDescr(rel->localrel));
EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1);
PushActiveSnapshot(GetTransactionSnapshot());
ExecOpenIndices(estate->es_result_relation_info, false);
/* Find the tuple using the replica identity index. */
@ -919,15 +927,10 @@ apply_handle_delete(StringInfo s)
}
/* Cleanup. */
ExecCloseIndices(estate->es_result_relation_info);
PopActiveSnapshot();
/* Handle queued AFTER triggers. */
AfterTriggerEndQuery(estate);
EvalPlanQualEnd(&epqstate);
ExecResetTupleTable(estate->es_tupleTable, false);
FreeExecutorState(estate);
ExecCloseIndices(estate->es_result_relation_info);
finish_estate(estate);
logicalrep_rel_close(rel, NoLock);
@ -950,7 +953,7 @@ apply_handle_truncate(StringInfo s)
List *relids = NIL;
List *relids_logged = NIL;
ListCell *lc;
LOCKMODE lockmode = AccessExclusiveLock;
LOCKMODE lockmode = AccessExclusiveLock;
ensure_transaction();