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

Commit1efc7e5

Browse files
committed
Fix problems with ParamListInfo serialization mechanism.
Commitd1b7c1f introduced a mechanismfor serializing a ParamListInfo structure to be passed to a parallelworker. However, this mechanism failed to handle external expandedvalues, as pointed out by Noah Misch. Repair.Moreover, plpgsql_param_fetch requires adjustment because theserialization mechanism needs it to skip evaluating unused parametersjust as we would do when it is called from copyParamList, but params== estate->paramLI in that case. To fix, make the bms_is_member testin that function unconditional.Finally, have setup_param_list set a new ParamListInfo field,paramMask, to the parameters actually used in the expression, so thatwe don't try to fetch those that are not needed when serializing aparameter list. This isn't necessary for correctness, but it makesthe performance of the parallel executor code comparable to what wedo for cases involving cursors.Design suggestions and extensive review by Noah Misch. Patch by me.
1 parentbf25fb2 commit1efc7e5

File tree

8 files changed

+89
-29
lines changed

8 files changed

+89
-29
lines changed

‎src/backend/commands/prepare.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,7 @@ EvaluateParams(PreparedStatement *pstmt, List *params,
392392
paramLI->parserSetup=NULL;
393393
paramLI->parserSetupArg=NULL;
394394
paramLI->numParams=num_params;
395+
paramLI->paramMask=NULL;
395396

396397
i=0;
397398
foreach(l,exprstates)

‎src/backend/executor/functions.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -910,6 +910,7 @@ postquel_sub_params(SQLFunctionCachePtr fcache,
910910
paramLI->parserSetup=NULL;
911911
paramLI->parserSetupArg=NULL;
912912
paramLI->numParams=nargs;
913+
paramLI->paramMask=NULL;
913914
fcache->paramLI=paramLI;
914915
}
915916
else

‎src/backend/executor/spi.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2330,6 +2330,7 @@ _SPI_convert_params(int nargs, Oid *argtypes,
23302330
paramLI->parserSetup=NULL;
23312331
paramLI->parserSetupArg=NULL;
23322332
paramLI->numParams=nargs;
2333+
paramLI->paramMask=NULL;
23332334

23342335
for (i=0;i<nargs;i++)
23352336
{

‎src/backend/nodes/params.c

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
#include"postgres.h"
1717

18+
#include"nodes/bitmapset.h"
1819
#include"nodes/params.h"
1920
#include"storage/shmem.h"
2021
#include"utils/datum.h"
@@ -50,6 +51,7 @@ copyParamList(ParamListInfo from)
5051
retval->parserSetup=NULL;
5152
retval->parserSetupArg=NULL;
5253
retval->numParams=from->numParams;
54+
retval->paramMask=NULL;
5355

5456
for (i=0;i<from->numParams;i++)
5557
{
@@ -58,6 +60,17 @@ copyParamList(ParamListInfo from)
5860
int16typLen;
5961
booltypByVal;
6062

63+
/* Ignore parameters we don't need, to save cycles and space. */
64+
if (retval->paramMask!=NULL&&
65+
!bms_is_member(i,retval->paramMask))
66+
{
67+
nprm->value= (Datum)0;
68+
nprm->isnull= true;
69+
nprm->pflags=0;
70+
nprm->ptype=InvalidOid;
71+
continue;
72+
}
73+
6174
/* give hook a chance in case parameter is dynamic */
6275
if (!OidIsValid(oprm->ptype)&&from->paramFetch!=NULL)
6376
(*from->paramFetch) (from,i+1);
@@ -90,19 +103,28 @@ EstimateParamListSpace(ParamListInfo paramLI)
90103
for (i=0;i<paramLI->numParams;i++)
91104
{
92105
ParamExternData*prm=&paramLI->params[i];
106+
OidtypeOid;
93107
int16typLen;
94108
booltypByVal;
95109

96-
/* give hook a chance in case parameter is dynamic */
97-
if (!OidIsValid(prm->ptype)&&paramLI->paramFetch!=NULL)
98-
(*paramLI->paramFetch) (paramLI,i+1);
110+
/* Ignore parameters we don't need, to save cycles and space. */
111+
if (paramLI->paramMask!=NULL&&
112+
!bms_is_member(i,paramLI->paramMask))
113+
typeOid=InvalidOid;
114+
else
115+
{
116+
/* give hook a chance in case parameter is dynamic */
117+
if (!OidIsValid(prm->ptype)&&paramLI->paramFetch!=NULL)
118+
(*paramLI->paramFetch) (paramLI,i+1);
119+
typeOid=prm->ptype;
120+
}
99121

100122
sz=add_size(sz,sizeof(Oid));/* space for type OID */
101123
sz=add_size(sz,sizeof(uint16));/* space for pflags */
102124

103125
/* space for datum/isnull */
104-
if (OidIsValid(prm->ptype))
105-
get_typlenbyval(prm->ptype,&typLen,&typByVal);
126+
if (OidIsValid(typeOid))
127+
get_typlenbyval(typeOid,&typLen,&typByVal);
106128
else
107129
{
108130
/* If no type OID, assume by-value, like copyParamList does. */
@@ -150,24 +172,33 @@ SerializeParamList(ParamListInfo paramLI, char **start_address)
150172
for (i=0;i<nparams;i++)
151173
{
152174
ParamExternData*prm=&paramLI->params[i];
175+
OidtypeOid;
153176
int16typLen;
154177
booltypByVal;
155178

156-
/* give hook a chance in case parameter is dynamic */
157-
if (!OidIsValid(prm->ptype)&&paramLI->paramFetch!=NULL)
158-
(*paramLI->paramFetch) (paramLI,i+1);
179+
/* Ignore parameters we don't need, to save cycles and space. */
180+
if (paramLI->paramMask!=NULL&&
181+
!bms_is_member(i,paramLI->paramMask))
182+
typeOid=InvalidOid;
183+
else
184+
{
185+
/* give hook a chance in case parameter is dynamic */
186+
if (!OidIsValid(prm->ptype)&&paramLI->paramFetch!=NULL)
187+
(*paramLI->paramFetch) (paramLI,i+1);
188+
typeOid=prm->ptype;
189+
}
159190

160191
/* Write type OID. */
161-
memcpy(*start_address,&prm->ptype,sizeof(Oid));
192+
memcpy(*start_address,&typeOid,sizeof(Oid));
162193
*start_address+=sizeof(Oid);
163194

164195
/* Write flags. */
165196
memcpy(*start_address,&prm->pflags,sizeof(uint16));
166197
*start_address+=sizeof(uint16);
167198

168199
/* Write datum/isnull. */
169-
if (OidIsValid(prm->ptype))
170-
get_typlenbyval(prm->ptype,&typLen,&typByVal);
200+
if (OidIsValid(typeOid))
201+
get_typlenbyval(typeOid,&typLen,&typByVal);
171202
else
172203
{
173204
/* If no type OID, assume by-value, like copyParamList does. */
@@ -209,6 +240,7 @@ RestoreParamList(char **start_address)
209240
paramLI->parserSetup=NULL;
210241
paramLI->parserSetupArg=NULL;
211242
paramLI->numParams=nparams;
243+
paramLI->paramMask=NULL;
212244

213245
for (i=0;i<nparams;i++)
214246
{

‎src/backend/tcop/postgres.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1629,6 +1629,7 @@ exec_bind_message(StringInfo input_message)
16291629
params->parserSetup=NULL;
16301630
params->parserSetupArg=NULL;
16311631
params->numParams=numParams;
1632+
params->paramMask=NULL;
16321633

16331634
for (paramno=0;paramno<numParams;paramno++)
16341635
{

‎src/backend/utils/adt/datum.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,11 @@ datumEstimateSpace(Datum value, bool isnull, bool typByVal, int typLen)
264264
/* no need to use add_size, can't overflow */
265265
if (typByVal)
266266
sz+=sizeof(Datum);
267+
elseif (VARATT_IS_EXTERNAL_EXPANDED(value))
268+
{
269+
ExpandedObjectHeader*eoh=DatumGetEOHP(value);
270+
sz+=EOH_get_flat_size(eoh);
271+
}
267272
else
268273
sz+=datumGetSize(value,typByVal,typLen);
269274
}
@@ -292,13 +297,19 @@ void
292297
datumSerialize(Datumvalue,boolisnull,booltypByVal,inttypLen,
293298
char**start_address)
294299
{
300+
ExpandedObjectHeader*eoh=NULL;
295301
intheader;
296302

297303
/* Write header word. */
298304
if (isnull)
299305
header=-2;
300306
elseif (typByVal)
301307
header=-1;
308+
elseif (VARATT_IS_EXTERNAL_EXPANDED(value))
309+
{
310+
eoh=DatumGetEOHP(value);
311+
header=EOH_get_flat_size(eoh);
312+
}
302313
else
303314
header=datumGetSize(value,typByVal,typLen);
304315
memcpy(*start_address,&header,sizeof(int));
@@ -312,6 +323,11 @@ datumSerialize(Datum value, bool isnull, bool typByVal, int typLen,
312323
memcpy(*start_address,&value,sizeof(Datum));
313324
*start_address+=sizeof(Datum);
314325
}
326+
elseif (eoh)
327+
{
328+
EOH_flatten_into(eoh, (void*)*start_address,header);
329+
*start_address+=header;
330+
}
315331
else
316332
{
317333
memcpy(*start_address,DatumGetPointer(value),header);

‎src/include/nodes/params.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
#ifndefPARAMS_H
1515
#definePARAMS_H
1616

17-
/* To avoid including a pile of parser headers, reference ParseState thus: */
17+
/* Forward declarations, to avoid including other headers */
18+
structBitmapset;
1819
structParseState;
1920

2021

@@ -71,6 +72,7 @@ typedef struct ParamListInfoData
7172
ParserSetupHookparserSetup;/* parser setup hook */
7273
void*parserSetupArg;
7374
intnumParams;/* number of ParamExternDatas following */
75+
structBitmapset*paramMask;/* if non-NULL, can ignore omitted params */
7476
ParamExternDataparams[FLEXIBLE_ARRAY_MEMBER];
7577
}ParamListInfoData;
7678

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

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3287,6 +3287,7 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
32873287
estate->paramLI->parserSetup= (ParserSetupHook)plpgsql_parser_setup;
32883288
estate->paramLI->parserSetupArg=NULL;/* filled during use */
32893289
estate->paramLI->numParams=estate->ndatums;
3290+
estate->paramLI->paramMask=NULL;
32903291
estate->params_dirty= false;
32913292

32923293
/* set up for use of appropriate simple-expression EState and cast hash */
@@ -5558,6 +5559,12 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
55585559
*/
55595560
paramLI->parserSetupArg= (void*)expr;
55605561

5562+
/*
5563+
* Allow parameters that aren't needed by this expression to be
5564+
* ignored.
5565+
*/
5566+
paramLI->paramMask=expr->paramnos;
5567+
55615568
/*
55625569
* Also make sure this is set before parser hooks need it. There is
55635570
* no need to save and restore, since the value is always correct once
@@ -5592,6 +5599,9 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
55925599
* shared param list, where it could get passed to some less-trusted function.
55935600
*
55945601
* Caller should pfree the result after use, if it's not NULL.
5602+
*
5603+
* XXX. Could we use ParamListInfo's new paramMask to avoid creating unshared
5604+
* parameter lists?
55955605
*/
55965606
staticParamListInfo
55975607
setup_unshared_param_list(PLpgSQL_execstate*estate,PLpgSQL_expr*expr)
@@ -5623,6 +5633,7 @@ setup_unshared_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
56235633
paramLI->parserSetup= (ParserSetupHook)plpgsql_parser_setup;
56245634
paramLI->parserSetupArg= (void*)expr;
56255635
paramLI->numParams=estate->ndatums;
5636+
paramLI->paramMask=NULL;
56265637

56275638
/*
56285639
* Instantiate values for "safe" parameters of the expression. We
@@ -5696,25 +5707,20 @@ plpgsql_param_fetch(ParamListInfo params, int paramid)
56965707
/* now we can access the target datum */
56975708
datum=estate->datums[dno];
56985709

5699-
/* need to behave slightly differently for shared and unshared arrays */
5700-
if (params!=estate->paramLI)
5701-
{
5702-
/*
5703-
* We're being called, presumably from copyParamList(), for cursor
5704-
* parameters. Since copyParamList() will try to materialize every
5705-
* single parameter slot, it's important to do nothing when asked for
5706-
* a datum that's not supposed to be used by this SQL expression.
5707-
* Otherwise we risk failures in exec_eval_datum(), not to mention
5708-
* possibly copying a lot more data than the cursor actually uses.
5709-
*/
5710-
if (!bms_is_member(dno,expr->paramnos))
5711-
return;
5712-
}
5713-
else
5710+
/*
5711+
* Since copyParamList() or SerializeParamList() will try to materialize
5712+
* every single parameter slot, it's important to do nothing when asked
5713+
* for a datum that's not supposed to be used by this SQL expression.
5714+
* Otherwise we risk failures in exec_eval_datum(), or copying a lot more
5715+
* data than necessary.
5716+
*/
5717+
if (!bms_is_member(dno,expr->paramnos))
5718+
return;
5719+
5720+
if (params==estate->paramLI)
57145721
{
57155722
/*
5716-
* Normal evaluation cases. We don't need to sanity-check dno, but we
5717-
* do need to mark the shared params array dirty if we're about to
5723+
* We need to mark the shared params array dirty if we're about to
57185724
* evaluate a resettable datum.
57195725
*/
57205726
switch (datum->dtype)

0 commit comments

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