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

Commit7efa841

Browse files
committed
Rethink plpgsql's way of handling SPI execution during an exception block.
We don't really want to start a new SPI connection, just keep using the oldone; otherwise we have memory management problems as illustrated byJohn Kennedy's bug report of today. This requires a bit of a hack toensure the SPI stack state is properly restored, but then again what wewere doing before was a hack too, strictly speaking. Add a regressiontest to cover this case.
1 parent2bb3bcf commit7efa841

File tree

5 files changed

+72
-20
lines changed

5 files changed

+72
-20
lines changed

‎src/backend/executor/spi.c‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.131 2004/10/13 01:25:10 neilc Exp $
11+
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.132 2004/11/16 18:10:13 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -270,6 +270,14 @@ SPI_pop(void)
270270
_SPI_curid--;
271271
}
272272

273+
/* Restore state of SPI stack after aborting a subtransaction */
274+
void
275+
SPI_restore_connection(void)
276+
{
277+
Assert(_SPI_connected >=0);
278+
_SPI_curid=_SPI_connected-1;
279+
}
280+
273281
/* Parse, plan, and execute a querystring */
274282
int
275283
SPI_execute(constchar*src,boolread_only,inttcount)

‎src/include/executor/spi.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
*
33
* spi.h
44
*
5-
* $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.49 2004/09/1616:58:40 tgl Exp $
5+
* $PostgreSQL: pgsql/src/include/executor/spi.h,v 1.50 2004/11/1618:10:13 tgl Exp $
66
*
77
*-------------------------------------------------------------------------
88
*/
@@ -81,6 +81,7 @@ extern intSPI_connect(void);
8181
externintSPI_finish(void);
8282
externvoidSPI_push(void);
8383
externvoidSPI_pop(void);
84+
externvoidSPI_restore_connection(void);
8485
externintSPI_execute(constchar*src,boolread_only,inttcount);
8586
externintSPI_execute_plan(void*plan,Datum*Values,constchar*Nulls,
8687
boolread_only,inttcount);

‎src/pl/plpgsql/src/pl_exec.c‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* procedural language
44
*
55
* IDENTIFICATION
6-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.120 2004/09/1616:58:44 tgl Exp $
6+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.121 2004/11/1618:10:14 tgl Exp $
77
*
88
* This software is copyrighted by Jan Wieck - Hamburg.
99
*
@@ -899,20 +899,11 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
899899
MemoryContextoldcontext=CurrentMemoryContext;
900900
ResourceOwneroldowner=CurrentResourceOwner;
901901
volatileboolcaught= false;
902-
intxrc;
903902

904-
/*
905-
* Start a subtransaction, and re-connect to SPI within it
906-
*/
907-
SPI_push();
908903
BeginInternalSubTransaction(NULL);
909904
/* Want to run statements inside function's memory context */
910905
MemoryContextSwitchTo(oldcontext);
911906

912-
if ((xrc=SPI_connect())!=SPI_OK_CONNECT)
913-
elog(ERROR,"SPI_connect failed: %s",
914-
SPI_result_code_string(xrc));
915-
916907
PG_TRY();
917908
{
918909
rc=exec_stmts(estate,block->body);
@@ -928,12 +919,17 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
928919
edata=CopyErrorData();
929920
FlushErrorState();
930921

931-
/* Abort the inner transaction(and inner SPI connection)*/
922+
/* Abort the inner transaction */
932923
RollbackAndReleaseCurrentSubTransaction();
933924
MemoryContextSwitchTo(oldcontext);
934925
CurrentResourceOwner=oldowner;
935926

936-
SPI_pop();
927+
/*
928+
* If AtEOSubXact_SPI() popped any SPI context of the subxact,
929+
* it will have left us in a disconnected state. We need this
930+
* hack to return to connected state.
931+
*/
932+
SPI_restore_connection();
937933

938934
/* Look for a matching exception handler */
939935
exceptions=block->exceptions;
@@ -960,15 +956,14 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
960956
/* Commit the inner transaction, return to outer xact context */
961957
if (!caught)
962958
{
963-
if ((xrc=SPI_finish())!=SPI_OK_FINISH)
964-
elog(ERROR,"SPI_finish failed: %s",
965-
SPI_result_code_string(xrc));
966-
967959
ReleaseCurrentSubTransaction();
968960
MemoryContextSwitchTo(oldcontext);
969961
CurrentResourceOwner=oldowner;
970-
971-
SPI_pop();
962+
/*
963+
* AtEOSubXact_SPI() should not have popped any SPI context,
964+
* but just in case it did, make sure we remain connected.
965+
*/
966+
SPI_restore_connection();
972967
}
973968
}
974969
else

‎src/test/regress/expected/plpgsql.out‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1931,6 +1931,36 @@ select * from foo;
19311931
20
19321932
(2 rows)
19331933

1934+
-- Test for pass-by-ref values being stored in proper context
1935+
create function test_variable_storage() returns text as $$
1936+
declare x text;
1937+
begin
1938+
x := '1234';
1939+
begin
1940+
x := x || '5678';
1941+
-- force error inside subtransaction SPI context
1942+
perform trap_zero_divide(-100);
1943+
exception
1944+
when others then
1945+
x := x || '9012';
1946+
end;
1947+
return x;
1948+
end$$ language plpgsql;
1949+
select test_variable_storage();
1950+
NOTICE: should see this
1951+
CONTEXT: SQL statement "SELECT trap_zero_divide(-100)"
1952+
PL/pgSQL function "test_variable_storage" line 7 at perform
1953+
NOTICE: should see this only if -100 <> 0
1954+
CONTEXT: SQL statement "SELECT trap_zero_divide(-100)"
1955+
PL/pgSQL function "test_variable_storage" line 7 at perform
1956+
NOTICE: should see this only if -100 fits in smallint
1957+
CONTEXT: SQL statement "SELECT trap_zero_divide(-100)"
1958+
PL/pgSQL function "test_variable_storage" line 7 at perform
1959+
test_variable_storage
1960+
-----------------------
1961+
123456789012
1962+
(1 row)
1963+
19341964
--
19351965
-- test foreign key error trapping
19361966
--

‎src/test/regress/sql/plpgsql.sql‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,6 +1696,24 @@ reset statement_timeout;
16961696

16971697
select*from foo;
16981698

1699+
-- Test for pass-by-ref values being stored in proper context
1700+
createfunctiontest_variable_storage() returnstextas $$
1701+
declare xtext;
1702+
begin
1703+
x :='1234';
1704+
begin
1705+
x := x||'5678';
1706+
-- force error inside subtransaction SPI context
1707+
perform trap_zero_divide(-100);
1708+
exception
1709+
when others then
1710+
x := x||'9012';
1711+
end;
1712+
return x;
1713+
end$$ language plpgsql;
1714+
1715+
select test_variable_storage();
1716+
16991717
--
17001718
-- test foreign key error trapping
17011719
--

0 commit comments

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