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

Commit1518d07

Browse files
committed
Fix crash when logical decoding is invoked from a PL function.
The logical decoding functions do BeginInternalSubTransaction andRollbackAndReleaseCurrentSubTransaction to clean up after themselves.It turns out that AtEOSubXact_SPI has an unrecognized assumption thatwe always need to cancel the active SPI operation in the SPI contextthat surrounds the subtransaction (if there is one). That's truewhen the RollbackAndReleaseCurrentSubTransaction call is coming fromthe SPI-using function itself, but not when it's happening insidesome unrelated function invoked by a SPI query. In practice theaffected callers are the various PLs.To fix, record the current subtransaction ID when we begin a SPIoperation, and clean up only if that ID is the subtransaction beingcanceled.Also, remove AtEOSubXact_SPI's assertion that it must have cleanedup the surrounding SPI context's active tuptable. That's provenwrong by the same test case.Also clarify (or, if you prefer, reinterpret) the calling conventionsfor _SPI_begin_call and _SPI_end_call. The memory context cleanupin the latter means that these have always had the flavor of a matchedresource-management pair, but they weren't documented that way before.Per report from Ben Chobot.Back-patch to 9.4 where logical decoding came in. In principle,the SPI changes should go all the way back, since the problem datesback to commit7ec1c5a. But given the lack of field complaintsit seems few people are using internal subtransactions in this way.So I don't feel a need to take any risks in 9.2/9.3.Discussion:https://postgr.es/m/73FBA179-C68C-4540-9473-71E865408B15@silentmedia.com
1 parent45866c7 commit1518d07

File tree

4 files changed

+70
-11
lines changed

4 files changed

+70
-11
lines changed

‎contrib/test_decoding/expected/decoding_into_rel.out

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,31 @@ SELECT * FROM changeresult;
5959

6060
DROP TABLE changeresult;
6161
DROP TABLE somechange;
62+
-- check calling logical decoding from pl/pgsql
63+
CREATE FUNCTION slot_changes_wrapper(slot_name name) RETURNS SETOF TEXT AS $$
64+
BEGIN
65+
RETURN QUERY
66+
SELECT data FROM pg_logical_slot_peek_changes(slot_name, NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
67+
END$$ LANGUAGE plpgsql;
68+
SELECT * FROM slot_changes_wrapper('regression_slot');
69+
slot_changes_wrapper
70+
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
71+
BEGIN
72+
table public.changeresult: INSERT: data[text]:'BEGIN'
73+
table public.changeresult: INSERT: data[text]:'table public.changeresult: INSERT: data[text]:''BEGIN'''
74+
table public.changeresult: INSERT: data[text]:'table public.changeresult: INSERT: data[text]:''table public.somechange: INSERT: id[integer]:1'''
75+
table public.changeresult: INSERT: data[text]:'table public.changeresult: INSERT: data[text]:''COMMIT'''
76+
table public.changeresult: INSERT: data[text]:'COMMIT'
77+
table public.changeresult: INSERT: data[text]:'BEGIN'
78+
table public.changeresult: INSERT: data[text]:'table public.changeresult: INSERT: data[text]:''BEGIN'''
79+
table public.changeresult: INSERT: data[text]:'table public.changeresult: INSERT: data[text]:''table public.changeresult: INSERT: data[text]:''''BEGIN'''''''
80+
table public.changeresult: INSERT: data[text]:'table public.changeresult: INSERT: data[text]:''table public.changeresult: INSERT: data[text]:''''table public.somechange: INSERT: id[integer]:1'''''''
81+
table public.changeresult: INSERT: data[text]:'table public.changeresult: INSERT: data[text]:''table public.changeresult: INSERT: data[text]:''''COMMIT'''''''
82+
table public.changeresult: INSERT: data[text]:'table public.changeresult: INSERT: data[text]:''COMMIT'''
83+
table public.changeresult: INSERT: data[text]:'COMMIT'
84+
COMMIT
85+
(14 rows)
86+
6287
SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
6388
data
6489
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

‎contrib/test_decoding/sql/decoding_into_rel.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,16 @@ INSERT INTO changeresult
2727
SELECT*FROM changeresult;
2828
DROPTABLE changeresult;
2929
DROPTABLE somechange;
30+
31+
-- check calling logical decoding from pl/pgsql
32+
CREATEFUNCTIONslot_changes_wrapper(slot_name name) RETURNS SETOFTEXTAS $$
33+
BEGIN
34+
RETURN QUERY
35+
SELECT dataFROM pg_logical_slot_peek_changes(slot_name,NULL,NULL,'include-xids','0','skip-empty-xacts','1');
36+
END$$ LANGUAGE plpgsql;
37+
38+
SELECT*FROM slot_changes_wrapper('regression_slot');
39+
3040
SELECT dataFROM pg_logical_slot_get_changes('regression_slot',NULL,NULL,'include-xids','0','skip-empty-xacts','1');
41+
3142
SELECT'stop'FROM pg_drop_replication_slot('regression_slot');

‎src/backend/executor/spi.c

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ static void _SPI_cursor_operation(Portal portal,
7171
staticSPIPlanPtr_SPI_make_plan_non_temp(SPIPlanPtrplan);
7272
staticSPIPlanPtr_SPI_save_plan(SPIPlanPtrplan);
7373

74-
staticint_SPI_begin_call(boolexecmem);
75-
staticint_SPI_end_call(boolprocmem);
74+
staticint_SPI_begin_call(booluse_exec);
75+
staticint_SPI_end_call(booluse_exec);
7676
staticMemoryContext_SPI_execmem(void);
7777
staticMemoryContext_SPI_procmem(void);
7878
staticbool_SPI_checktuples(void);
@@ -118,6 +118,7 @@ SPI_connect(void)
118118
_SPI_current->processed=0;
119119
_SPI_current->lastoid=InvalidOid;
120120
_SPI_current->tuptable=NULL;
121+
_SPI_current->execSubid=InvalidSubTransactionId;
121122
slist_init(&_SPI_current->tuptables);
122123
_SPI_current->procCxt=NULL;/* in case we fail to create 'em */
123124
_SPI_current->execCxt=NULL;
@@ -149,7 +150,7 @@ SPI_finish(void)
149150
{
150151
intres;
151152

152-
res=_SPI_begin_call(false);/*live in procedure memory */
153+
res=_SPI_begin_call(false);/*just check we're connected */
153154
if (res<0)
154155
returnres;
155156

@@ -268,8 +269,15 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
268269
{
269270
slist_mutable_itersiter;
270271

271-
/* free Executor memory the same as _SPI_end_call would do */
272-
MemoryContextResetAndDeleteChildren(_SPI_current->execCxt);
272+
/*
273+
* Throw away executor state if current executor operation was started
274+
* within current subxact (essentially, force a _SPI_end_call(true)).
275+
*/
276+
if (_SPI_current->execSubid >=mySubid)
277+
{
278+
_SPI_current->execSubid=InvalidSubTransactionId;
279+
MemoryContextResetAndDeleteChildren(_SPI_current->execCxt);
280+
}
273281

274282
/* throw away any tuple tables created within current subxact */
275283
slist_foreach_modify(siter,&_SPI_current->tuptables)
@@ -293,8 +301,6 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
293301
MemoryContextDelete(tuptable->tuptabcxt);
294302
}
295303
}
296-
/* in particular we should have gotten rid of any in-progress table */
297-
Assert(_SPI_current->tuptable==NULL);
298304
}
299305
}
300306

@@ -2446,30 +2452,44 @@ _SPI_procmem(void)
24462452

24472453
/*
24482454
* _SPI_begin_call: begin a SPI operation within a connected procedure
2455+
*
2456+
* use_exec is true if we intend to make use of the procedure's execCxt
2457+
* during this SPI operation. We'll switch into that context, and arrange
2458+
* for it to be cleaned up at _SPI_end_call or if an error occurs.
24492459
*/
24502460
staticint
2451-
_SPI_begin_call(boolexecmem)
2461+
_SPI_begin_call(booluse_exec)
24522462
{
24532463
if (_SPI_current==NULL)
24542464
returnSPI_ERROR_UNCONNECTED;
24552465

2456-
if (execmem)/* switch to the Executor memory context */
2466+
if (use_exec)
2467+
{
2468+
/* remember when the Executor operation started */
2469+
_SPI_current->execSubid=GetCurrentSubTransactionId();
2470+
/* switch to the Executor memory context */
24572471
_SPI_execmem();
2472+
}
24582473

24592474
return0;
24602475
}
24612476

24622477
/*
24632478
* _SPI_end_call: end a SPI operation within a connected procedure
24642479
*
2480+
* use_exec must be the same as in the previous _SPI_begin_call
2481+
*
24652482
* Note: this currently has no failure return cases, so callers don't check
24662483
*/
24672484
staticint
2468-
_SPI_end_call(boolprocmem)
2485+
_SPI_end_call(booluse_exec)
24692486
{
2470-
if (procmem)/* switch to the procedure memory context */
2487+
if (use_exec)
24712488
{
2489+
/* switch to the procedure memory context */
24722490
_SPI_procmem();
2491+
/* mark Executor context no longer in use */
2492+
_SPI_current->execSubid=InvalidSubTransactionId;
24732493
/* and free Executor memory */
24742494
MemoryContextResetAndDeleteChildren(_SPI_current->execCxt);
24752495
}

‎src/include/executor/spi_priv.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ typedef struct
2626
Oidlastoid;
2727
SPITupleTable*tuptable;/* tuptable currently being built */
2828

29+
/* subtransaction in which current Executor call was started */
30+
SubTransactionIdexecSubid;
31+
2932
/* resources of this execution context */
3033
slist_headtuptables;/* list of all live SPITupleTables */
3134
MemoryContextprocCxt;/* procedure context */

0 commit comments

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