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

Commit83604cc

Browse files
committed
Repair unsafe use of shared typecast-lookup table in plpgsql DO blocks.
DO blocks use private simple_eval_estates to avoid intra-transaction memoryleakage, cf commitc7b849a. I had forgotten about that while writingcommit0fc94a5, but it means that expression execution trees createdwithin a DO block disappear immediately on exiting the DO block, and hencecan't safely be linked into plpgsql's session-wide cast hash table.To fix, give a DO block a private cast hash table to go with its privatesimple_eval_estate. This is less efficient than one could wish, sinceDO blocks can no longer share any cast lookup work with other plpgsqlexecution, but it shouldn't be too bad; in any case it's no worse thanwhat happened in DO blocks before commit0fc94a5.Per bug #13571 from Feike Steenbergen. Preliminary analysis byOleksandr Shulgin.
1 parente95126c commit83604cc

File tree

4 files changed

+71
-28
lines changed

4 files changed

+71
-28
lines changed

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

Lines changed: 51 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ static SimpleEcontextStackEntry *simple_econtext_stack = NULL;
9898
*
9999
* The evaluation state trees (cast_exprstate) are managed in the same way as
100100
* simple expressions (i.e., we assume cast expressions are always simple).
101+
*
102+
* As with simple expressions, DO blocks don't use the shared hash table but
103+
* must have their own. This isn't ideal, but we don't want to deal with
104+
* multiple simple_eval_estates within a DO block.
101105
*/
102106
typedefstruct/* lookup key for cast info */
103107
{
@@ -118,8 +122,8 @@ typedef struct/* cast_hash table entry */
118122
LocalTransactionIdcast_lxid;
119123
}plpgsql_CastHashEntry;
120124

121-
staticMemoryContextcast_hash_context=NULL;
122-
staticHTAB*cast_hash=NULL;
125+
staticMemoryContextshared_cast_context=NULL;
126+
staticHTAB*shared_cast_hash=NULL;
123127

124128
/************************************************************
125129
* Local function forward declarations
@@ -287,7 +291,9 @@ static char *format_preparedparamsdata(PLpgSQL_execstate *estate,
287291
* difference that this code is aware of is that for a DO block, we want
288292
* to use a private simple_eval_estate, which is created and passed in by
289293
* the caller. For regular functions, pass NULL, which implies using
290-
* shared_simple_eval_estate.
294+
* shared_simple_eval_estate. (When using a private simple_eval_estate,
295+
* we must also use a private cast hashtable, but that's taken care of
296+
* within plpgsql_estate_setup.)
291297
* ----------
292298
*/
293299
Datum
@@ -3271,6 +3277,8 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
32713277
ReturnSetInfo*rsi,
32723278
EState*simple_eval_estate)
32733279
{
3280+
HASHCTLctl;
3281+
32743282
/* this link will be restored at exit from plpgsql_call_handler */
32753283
func->cur_estate=estate;
32763284

@@ -3319,11 +3327,44 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
33193327
estate->paramLI->numParams=estate->ndatums;
33203328
estate->params_dirty= false;
33213329

3322-
/* set up for use of appropriate simple-expression EState */
3330+
/* set up for use of appropriate simple-expression EStateand cast hash*/
33233331
if (simple_eval_estate)
3332+
{
33243333
estate->simple_eval_estate=simple_eval_estate;
3334+
/* Private cast hash just lives in function's main context */
3335+
memset(&ctl,0,sizeof(ctl));
3336+
ctl.keysize=sizeof(plpgsql_CastHashKey);
3337+
ctl.entrysize=sizeof(plpgsql_CastHashEntry);
3338+
ctl.hcxt=CurrentMemoryContext;
3339+
estate->cast_hash=hash_create("PLpgSQL private cast cache",
3340+
16,/* start small and extend */
3341+
&ctl,
3342+
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
3343+
estate->cast_hash_context=CurrentMemoryContext;
3344+
}
33253345
else
3346+
{
33263347
estate->simple_eval_estate=shared_simple_eval_estate;
3348+
/* Create the session-wide cast-info hash table if we didn't already */
3349+
if (shared_cast_hash==NULL)
3350+
{
3351+
shared_cast_context=AllocSetContextCreate(TopMemoryContext,
3352+
"PLpgSQL cast info",
3353+
ALLOCSET_DEFAULT_MINSIZE,
3354+
ALLOCSET_DEFAULT_INITSIZE,
3355+
ALLOCSET_DEFAULT_MAXSIZE);
3356+
memset(&ctl,0,sizeof(ctl));
3357+
ctl.keysize=sizeof(plpgsql_CastHashKey);
3358+
ctl.entrysize=sizeof(plpgsql_CastHashEntry);
3359+
ctl.hcxt=shared_cast_context;
3360+
shared_cast_hash=hash_create("PLpgSQL cast cache",
3361+
16,/* start small and extend */
3362+
&ctl,
3363+
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
3364+
}
3365+
estate->cast_hash=shared_cast_hash;
3366+
estate->cast_hash_context=shared_cast_context;
3367+
}
33273368

33283369
estate->eval_tuptable=NULL;
33293370
estate->eval_processed=0;
@@ -6111,32 +6152,12 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
61116152
LocalTransactionIdcurlxid;
61126153
MemoryContextoldcontext;
61136154

6114-
/* Create the session-wide cast-info hash table if we didn't already */
6115-
if (cast_hash==NULL)
6116-
{
6117-
HASHCTLctl;
6118-
6119-
cast_hash_context=AllocSetContextCreate(TopMemoryContext,
6120-
"PLpgSQL cast info",
6121-
ALLOCSET_DEFAULT_MINSIZE,
6122-
ALLOCSET_DEFAULT_INITSIZE,
6123-
ALLOCSET_DEFAULT_MAXSIZE);
6124-
memset(&ctl,0,sizeof(ctl));
6125-
ctl.keysize=sizeof(plpgsql_CastHashKey);
6126-
ctl.entrysize=sizeof(plpgsql_CastHashEntry);
6127-
ctl.hcxt=cast_hash_context;
6128-
cast_hash=hash_create("PLpgSQL cast cache",
6129-
16,/* start small and extend */
6130-
&ctl,
6131-
HASH_ELEM |HASH_BLOBS |HASH_CONTEXT);
6132-
}
6133-
61346155
/* Look for existing entry */
61356156
cast_key.srctype=srctype;
61366157
cast_key.dsttype=dsttype;
61376158
cast_key.srctypmod=srctypmod;
61386159
cast_key.dsttypmod=dsttypmod;
6139-
cast_entry= (plpgsql_CastHashEntry*)hash_search(cast_hash,
6160+
cast_entry= (plpgsql_CastHashEntry*)hash_search(estate->cast_hash,
61406161
(void*)&cast_key,
61416162
HASH_FIND,NULL);
61426163

@@ -6221,15 +6242,15 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
62216242
cast_expr= (Node*)expression_planner((Expr*)cast_expr);
62226243

62236244
/* Now copy the tree into cast_hash_context */
6224-
MemoryContextSwitchTo(cast_hash_context);
6245+
MemoryContextSwitchTo(estate->cast_hash_context);
62256246

62266247
cast_expr=copyObject(cast_expr);
62276248
}
62286249

62296250
MemoryContextSwitchTo(oldcontext);
62306251

62316252
/* Now we can fill in a hashtable entry. */
6232-
cast_entry= (plpgsql_CastHashEntry*)hash_search(cast_hash,
6253+
cast_entry= (plpgsql_CastHashEntry*)hash_search(estate->cast_hash,
62336254
(void*)&cast_key,
62346255
HASH_ENTER,&found);
62356256
Assert(!found);/* wasn't there a moment ago */
@@ -6251,7 +6272,9 @@ get_cast_hashentry(PLpgSQL_execstate *estate,
62516272
* executions. (We will leak some memory intra-transaction if that
62526273
* happens a lot, but we don't expect it to.) It's okay to update the
62536274
* hash table with the new tree because all plpgsql functions within a
6254-
* given transaction share the same simple_eval_estate.
6275+
* given transaction share the same simple_eval_estate. (Well, regular
6276+
* functions do; DO blocks have private simple_eval_estates, and private
6277+
* cast hash tables to go with them.)
62556278
*/
62566279
curlxid=MyProc->lxid;
62576280
if (cast_entry->cast_lxid!=curlxid||cast_entry->cast_in_use)

‎src/pl/plpgsql/src/plpgsql.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,10 @@ typedef struct PLpgSQL_execstate
801801
/* EState to use for "simple" expression evaluation */
802802
EState*simple_eval_estate;
803803

804+
/* Lookup table to use for executing type casts */
805+
HTAB*cast_hash;
806+
MemoryContextcast_hash_context;
807+
804808
/* temporary state for results from evaluation of query or expr */
805809
SPITupleTable*eval_tuptable;
806810
uint32eval_processed;

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4764,6 +4764,13 @@ commit;
47644764
drop function cast_invoker(integer);
47654765
drop function sql_to_date(integer) cascade;
47664766
NOTICE: drop cascades to cast from integer to date
4767+
-- Test handling of cast cache inside DO blocks
4768+
-- (to check the original crash case, this must be a cast not previously
4769+
-- used in this session)
4770+
begin;
4771+
do $$ declare x text[]; begin x := '{1.23, 4.56}'::numeric[]; end $$;
4772+
do $$ declare x text[]; begin x := '{1.23, 4.56}'::numeric[]; end $$;
4773+
end;
47674774
-- Test for consistent reporting of error context
47684775
create function fail() returns int language plpgsql as $$
47694776
begin

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3836,6 +3836,15 @@ commit;
38363836
dropfunction cast_invoker(integer);
38373837
dropfunction sql_to_date(integer) cascade;
38383838

3839+
-- Test handling of cast cache inside DO blocks
3840+
-- (to check the original crash case, this must be a cast not previously
3841+
-- used in this session)
3842+
3843+
begin;
3844+
do $$ declare xtext[];begin x :='{1.23, 4.56}'::numeric[]; end $$;
3845+
do $$ declare xtext[];begin x :='{1.23, 4.56}'::numeric[]; end $$;
3846+
end;
3847+
38393848
-- Test for consistent reporting of error context
38403849

38413850
createfunctionfail() returnsint language plpgsqlas $$

0 commit comments

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