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

Commit9aab83f

Browse files
committed
Redesign get_attstatsslot()/free_attstatsslot() for more safety and speed.
The mess cleaned up in commitda07596 is clear evidence that it's abug hazard to expect the caller of get_attstatsslot()/free_attstatsslot()to provide the correct type OID for the array elements in the slot.Moreover, we weren't even getting any performance benefit from that,since get_attstatsslot() was extracting the real type OID from the arrayanyway. So we ought to get rid of that requirement; indeed, it wouldmake more sense for get_attstatsslot() to pass back the type OID it found,in case the caller isn't sure what to expect, which is likely in binary-compatible-operator cases.Another problem with the current implementation is that if the stats arrayelement type is pass-by-reference, we incur a palloc/memcpy/pfree cyclefor each element. That seemed acceptable when the code was written becausewe were targeting O(10) array sizes --- but these days, stats arrays arealmost always bigger than that, sometimes much bigger. We can save asignificant number of cycles by doing one palloc/memcpy/pfree of the wholearray. Indeed, in the now-probably-common case where the array is toasted,that happens anyway so this method is basically free. (Note: although thecatcache code will inline any out-of-line toasted values, it doesn'tdecompress them. At the other end of the size range, it doesn't expandshort-header datums either. In either case, DatumGetArrayTypeP would haveto make a copy. We do end up using an extra array copy step if the elementtype is pass-by-value and the array length is neither small enough for ashort header nor large enough to have suffered compression. But thatseems like a very acceptable price for winning in pass-by-ref cases.)Hence, redesign to take these insights into account. While at it,convert to an API in which we fill a struct rather than passing a bunchof pointers to individual output arguments. That will make it lesspainful if we ever want further expansion of what get_attstatsslot canpass back.It's certainly arguable that this is new development and not something topush post-feature-freeze. However, I view it as primarily bug-proofingand therefore something that's better to have sooner not later. Sincewe aren't quite at beta phase yet, let's put it in.Discussion:https://postgr.es/m/16364.1494520862@sss.pgh.pa.us
1 parent1848b73 commit9aab83f

File tree

10 files changed

+436
-601
lines changed

10 files changed

+436
-601
lines changed

‎contrib/intarray/_int_selfuncs.c

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,7 @@ _int_matchsel(PG_FUNCTION_ARGS)
136136
intnmcelems=0;
137137
float4minfreq=0.0;
138138
float4nullfrac=0.0;
139-
Form_pg_statisticstats;
140-
Datum*values=NULL;
141-
intnvalues=0;
142-
float4*numbers=NULL;
143-
intnnumbers=0;
139+
AttStatsSlotsslot;
144140

145141
/*
146142
* If expression is not "variable @@ something" or "something @@ variable"
@@ -193,36 +189,39 @@ _int_matchsel(PG_FUNCTION_ARGS)
193189
*/
194190
if (HeapTupleIsValid(vardata.statsTuple))
195191
{
192+
Form_pg_statisticstats;
193+
196194
stats= (Form_pg_statistic)GETSTRUCT(vardata.statsTuple);
197195
nullfrac=stats->stanullfrac;
198196

199197
/*
200198
* For an int4 array, the default array type analyze function will
201199
* collect a Most Common Elements list, which is an array of int4s.
202200
*/
203-
if (get_attstatsslot(vardata.statsTuple,
204-
INT4OID,-1,
201+
if (get_attstatsslot(&sslot,vardata.statsTuple,
205202
STATISTIC_KIND_MCELEM,InvalidOid,
206-
NULL,
207-
&values,&nvalues,
208-
&numbers,&nnumbers))
203+
ATTSTATSSLOT_VALUES |ATTSTATSSLOT_NUMBERS))
209204
{
205+
Assert(sslot.valuetype==INT4OID);
206+
210207
/*
211208
* There should be three more Numbers than Values, because the
212209
* last three (for intarray) cells are taken for minimal, maximal
213210
* and nulls frequency. Punt if not.
214211
*/
215-
if (nnumbers==nvalues+3)
212+
if (sslot.nnumbers==sslot.nvalues+3)
216213
{
217214
/* Grab the lowest frequency. */
218-
minfreq=numbers[nnumbers- (nnumbers-nvalues)];
215+
minfreq=sslot.numbers[sslot.nnumbers- (sslot.nnumbers-sslot.nvalues)];
219216

220-
mcelems=values;
221-
mcefreqs=numbers;
222-
nmcelems=nvalues;
217+
mcelems=sslot.values;
218+
mcefreqs=sslot.numbers;
219+
nmcelems=sslot.nvalues;
223220
}
224221
}
225222
}
223+
else
224+
memset(&sslot,0,sizeof(sslot));
226225

227226
/* Process the logical expression in the query, using the stats */
228227
selec=int_query_opr_selec(GETQUERY(query)+query->size-1,
@@ -231,7 +230,7 @@ _int_matchsel(PG_FUNCTION_ARGS)
231230
/* MCE stats count only non-null rows, so adjust for null rows. */
232231
selec *= (1.0-nullfrac);
233232

234-
free_attstatsslot(INT4OID,values,nvalues,numbers,nnumbers);
233+
free_attstatsslot(&sslot);
235234
ReleaseVariableStats(vardata);
236235

237236
CLAMP_PROBABILITY(selec);

‎src/backend/executor/nodeHash.c

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1283,10 +1283,7 @@ static void
12831283
ExecHashBuildSkewHash(HashJoinTablehashtable,Hash*node,intmcvsToUse)
12841284
{
12851285
HeapTupleData*statsTuple;
1286-
Datum*values;
1287-
intnvalues;
1288-
float4*numbers;
1289-
intnnumbers;
1286+
AttStatsSlotsslot;
12901287

12911288
/* Do nothing if planner didn't identify the outer relation's join key */
12921289
if (!OidIsValid(node->skewTable))
@@ -1305,19 +1302,17 @@ ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse)
13051302
if (!HeapTupleIsValid(statsTuple))
13061303
return;
13071304

1308-
if (get_attstatsslot(statsTuple,node->skewColType,node->skewColTypmod,
1305+
if (get_attstatsslot(&sslot,statsTuple,
13091306
STATISTIC_KIND_MCV,InvalidOid,
1310-
NULL,
1311-
&values,&nvalues,
1312-
&numbers,&nnumbers))
1307+
ATTSTATSSLOT_VALUES |ATTSTATSSLOT_NUMBERS))
13131308
{
13141309
doublefrac;
13151310
intnbuckets;
13161311
FmgrInfo*hashfunctions;
13171312
inti;
13181313

1319-
if (mcvsToUse>nvalues)
1320-
mcvsToUse=nvalues;
1314+
if (mcvsToUse>sslot.nvalues)
1315+
mcvsToUse=sslot.nvalues;
13211316

13221317
/*
13231318
* Calculate the expected fraction of outer relation that will
@@ -1326,11 +1321,10 @@ ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse)
13261321
*/
13271322
frac=0;
13281323
for (i=0;i<mcvsToUse;i++)
1329-
frac+=numbers[i];
1324+
frac+=sslot.numbers[i];
13301325
if (frac<SKEW_MIN_OUTER_FRACTION)
13311326
{
1332-
free_attstatsslot(node->skewColType,
1333-
values,nvalues,numbers,nnumbers);
1327+
free_attstatsslot(&sslot);
13341328
ReleaseSysCache(statsTuple);
13351329
return;
13361330
}
@@ -1392,7 +1386,7 @@ ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse)
13921386
intbucket;
13931387

13941388
hashvalue=DatumGetUInt32(FunctionCall1(&hashfunctions[0],
1395-
values[i]));
1389+
sslot.values[i]));
13961390

13971391
/*
13981392
* While we have not hit a hole in the hashtable and have not hit
@@ -1426,8 +1420,7 @@ ExecHashBuildSkewHash(HashJoinTable hashtable, Hash *node, int mcvsToUse)
14261420
hashtable->spacePeak=hashtable->spaceUsed;
14271421
}
14281422

1429-
free_attstatsslot(node->skewColType,
1430-
values,nvalues,numbers,nnumbers);
1423+
free_attstatsslot(&sslot);
14311424
}
14321425

14331426
ReleaseSysCache(statsTuple);

‎src/backend/tsearch/ts_selfuncs.c

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -163,28 +163,22 @@ tsquerysel(VariableStatData *vardata, Datum constval)
163163
if (HeapTupleIsValid(vardata->statsTuple))
164164
{
165165
Form_pg_statisticstats;
166-
Datum*values;
167-
intnvalues;
168-
float4*numbers;
169-
intnnumbers;
166+
AttStatsSlotsslot;
170167

171168
stats= (Form_pg_statistic)GETSTRUCT(vardata->statsTuple);
172169

173170
/* MCELEM will be an array of TEXT elements for a tsvector column */
174-
if (get_attstatsslot(vardata->statsTuple,
175-
TEXTOID,-1,
171+
if (get_attstatsslot(&sslot,vardata->statsTuple,
176172
STATISTIC_KIND_MCELEM,InvalidOid,
177-
NULL,
178-
&values,&nvalues,
179-
&numbers,&nnumbers))
173+
ATTSTATSSLOT_VALUES |ATTSTATSSLOT_NUMBERS))
180174
{
181175
/*
182176
* There is a most-common-elements slot for the tsvector Var, so
183177
* use that.
184178
*/
185-
selec=mcelem_tsquery_selec(query,values,nvalues,
186-
numbers,nnumbers);
187-
free_attstatsslot(TEXTOID,values,nvalues,numbers,nnumbers);
179+
selec=mcelem_tsquery_selec(query,sslot.values,sslot.nvalues,
180+
sslot.numbers,sslot.nnumbers);
181+
free_attstatsslot(&sslot);
188182
}
189183
else
190184
{

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

Lines changed: 31 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -137,58 +137,49 @@ scalararraysel_containment(PlannerInfo *root,
137137
statistic_proc_security_check(&vardata,cmpfunc->fn_oid))
138138
{
139139
Form_pg_statisticstats;
140-
Datum*values;
141-
intnvalues;
142-
float4*numbers;
143-
intnnumbers;
144-
float4*hist;
145-
intnhist;
140+
AttStatsSlotsslot;
141+
AttStatsSlothslot;
146142

147143
stats= (Form_pg_statistic)GETSTRUCT(vardata.statsTuple);
148144

149145
/* MCELEM will be an array of same type as element */
150-
if (get_attstatsslot(vardata.statsTuple,
151-
elemtype,vardata.atttypmod,
146+
if (get_attstatsslot(&sslot,vardata.statsTuple,
152147
STATISTIC_KIND_MCELEM,InvalidOid,
153-
NULL,
154-
&values,&nvalues,
155-
&numbers,&nnumbers))
148+
ATTSTATSSLOT_VALUES |ATTSTATSSLOT_NUMBERS))
156149
{
157150
/* For ALL case, also get histogram of distinct-element counts */
158151
if (useOr||
159-
!get_attstatsslot(vardata.statsTuple,
160-
elemtype,vardata.atttypmod,
152+
!get_attstatsslot(&hslot,vardata.statsTuple,
161153
STATISTIC_KIND_DECHIST,InvalidOid,
162-
NULL,
163-
NULL,NULL,
164-
&hist,&nhist))
165-
{
166-
hist=NULL;
167-
nhist=0;
168-
}
154+
ATTSTATSSLOT_NUMBERS))
155+
memset(&hslot,0,sizeof(hslot));
169156

170157
/*
171158
* For = ANY, estimate as var @> ARRAY[const].
172159
*
173160
* For = ALL, estimate as var <@ ARRAY[const].
174161
*/
175162
if (useOr)
176-
selec=mcelem_array_contain_overlap_selec(values,nvalues,
177-
numbers,nnumbers,
163+
selec=mcelem_array_contain_overlap_selec(sslot.values,
164+
sslot.nvalues,
165+
sslot.numbers,
166+
sslot.nnumbers,
178167
&constval,1,
179168
OID_ARRAY_CONTAINS_OP,
180169
cmpfunc);
181170
else
182-
selec=mcelem_array_contained_selec(values,nvalues,
183-
numbers,nnumbers,
171+
selec=mcelem_array_contained_selec(sslot.values,
172+
sslot.nvalues,
173+
sslot.numbers,
174+
sslot.nnumbers,
184175
&constval,1,
185-
hist,nhist,
176+
hslot.numbers,
177+
hslot.nnumbers,
186178
OID_ARRAY_CONTAINED_OP,
187179
cmpfunc);
188180

189-
if (hist)
190-
free_attstatsslot(elemtype,NULL,0,hist,nhist);
191-
free_attstatsslot(elemtype,values,nvalues,numbers,nnumbers);
181+
free_attstatsslot(&hslot);
182+
free_attstatsslot(&sslot);
192183
}
193184
else
194185
{
@@ -369,49 +360,35 @@ calc_arraycontsel(VariableStatData *vardata, Datum constval,
369360
statistic_proc_security_check(vardata,cmpfunc->fn_oid))
370361
{
371362
Form_pg_statisticstats;
372-
Datum*values;
373-
intnvalues;
374-
float4*numbers;
375-
intnnumbers;
376-
float4*hist;
377-
intnhist;
363+
AttStatsSlotsslot;
364+
AttStatsSlothslot;
378365

379366
stats= (Form_pg_statistic)GETSTRUCT(vardata->statsTuple);
380367

381368
/* MCELEM will be an array of same type as column */
382-
if (get_attstatsslot(vardata->statsTuple,
383-
elemtype,vardata->atttypmod,
369+
if (get_attstatsslot(&sslot,vardata->statsTuple,
384370
STATISTIC_KIND_MCELEM,InvalidOid,
385-
NULL,
386-
&values,&nvalues,
387-
&numbers,&nnumbers))
371+
ATTSTATSSLOT_VALUES |ATTSTATSSLOT_NUMBERS))
388372
{
389373
/*
390374
* For "array <@ const" case we also need histogram of distinct
391375
* element counts.
392376
*/
393377
if (operator!=OID_ARRAY_CONTAINED_OP||
394-
!get_attstatsslot(vardata->statsTuple,
395-
elemtype,vardata->atttypmod,
378+
!get_attstatsslot(&hslot,vardata->statsTuple,
396379
STATISTIC_KIND_DECHIST,InvalidOid,
397-
NULL,
398-
NULL,NULL,
399-
&hist,&nhist))
400-
{
401-
hist=NULL;
402-
nhist=0;
403-
}
380+
ATTSTATSSLOT_NUMBERS))
381+
memset(&hslot,0,sizeof(hslot));
404382

405383
/* Use the most-common-elements slot for the array Var. */
406384
selec=mcelem_array_selec(array,typentry,
407-
values,nvalues,
408-
numbers,nnumbers,
409-
hist,nhist,
385+
sslot.values,sslot.nvalues,
386+
sslot.numbers,sslot.nnumbers,
387+
hslot.numbers,hslot.nnumbers,
410388
operator,cmpfunc);
411389

412-
if (hist)
413-
free_attstatsslot(elemtype,NULL,0,hist,nhist);
414-
free_attstatsslot(elemtype,values,nvalues,numbers,nnumbers);
390+
free_attstatsslot(&hslot);
391+
free_attstatsslot(&sslot);
415392
}
416393
else
417394
{

0 commit comments

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