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

Commitbb5e312

Browse files
committed
Repair bugs in GiST page splitting code for multi-column indexes.
When considering a non-last column in a multi-column GiST index,gistsplit.c tries to improve on the split chosen by the opclass-specificpickSplit function by considering penalties for the next column. However,there were two bugs in this code: it failed to recompute the union keys forthe leftmost index columns, even though these might well change afterreassigning tuples; and it included the old union keys in the recomputationfor the columns it did recompute, so that those keys couldn't get smallereven if they should. The first problem could result in an invalid indexin which searches wouldn't find index entries that are in fact present;the second would make the index less efficient to search.Both of these errors were caused by misuse of gistMakeUnionItVec, whoseAPI was designed in a way that just begged such errors to be made. Thereis no situation in which it's safe or useful to compute the union keys fora subset of the index columns, and there is no caller that wants anyprevious union keys to be included in the computation; so the undocumentedchoice to treat the union keys as in/out rather than pure output parametersis a waste of code as well as being dangerous.Hence, rather than just making a minimal patch, I've changed the API ofgistMakeUnionItVec to remove the "startkey" parameter (it now alwaysprocesses all index columns) and treat the attr/isnull arrays as purelyoutput parameters.In passing, also get rid of a couple of unnecessary and dangerous usesof static variables in gistutil.c. It's remarkable that the one ingistMakeUnionKey hasn't given us portability troubles before now, becausein addition to posing a re-entrancy hazard, it was unsafely assuming thata static char[] array would have at least Datum alignment.Per investigation of a trouble report from Tomas Vondra. (There are alsosome bugs in contrib/btree_gist to be fixed, but that seems like materialfor a separate patch.) Back-patch to all supported branches.
1 parentd6f9b2c commitbb5e312

File tree

3 files changed

+47
-51
lines changed

3 files changed

+47
-51
lines changed

‎src/backend/access/gist/gistsplit.c

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,21 +19,21 @@
1919

2020
typedefstruct
2121
{
22-
Datum*attr;
23-
intlen;
2422
OffsetNumber*entries;
23+
intlen;
24+
Datum*attr;
2525
bool*isnull;
2626
bool*equiv;
2727
}GistSplitUnion;
2828

2929

3030
/*
31-
*Forms unions of subkeys after page split,but
32-
*uses only tuples aren'tingroups of equalent tuples
31+
*Form unions of subkeys afterapage split,ignoring any tuples
32+
*that are markedingsvp->equiv[]
3333
*/
3434
staticvoid
3535
gistunionsubkeyvec(GISTSTATE*giststate,IndexTuple*itvec,
36-
GistSplitUnion*gsvp,intstartkey)
36+
GistSplitUnion*gsvp)
3737
{
3838
IndexTuple*cleanedItVec;
3939
inti,
@@ -49,35 +49,36 @@ gistunionsubkeyvec(GISTSTATE *giststate, IndexTuple *itvec,
4949
cleanedItVec[cleanedLen++]=itvec[gsvp->entries[i]-1];
5050
}
5151

52-
gistMakeUnionItVec(giststate,cleanedItVec,cleanedLen,startkey,
52+
gistMakeUnionItVec(giststate,cleanedItVec,cleanedLen,
5353
gsvp->attr,gsvp->isnull);
5454

5555
pfree(cleanedItVec);
5656
}
5757

5858
/*
59-
* unions subkeys for after user picksplit over attno-1 column
59+
* Recompute unions of subkeys after a page split, ignoring any tuples
60+
* that are marked in spl->spl_equiv[]
6061
*/
6162
staticvoid
62-
gistunionsubkey(GISTSTATE*giststate,IndexTuple*itvec,GistSplitVector*spl,intattno)
63+
gistunionsubkey(GISTSTATE*giststate,IndexTuple*itvec,GistSplitVector*spl)
6364
{
6465
GistSplitUniongsvp;
6566

6667
gsvp.equiv=spl->spl_equiv;
6768

68-
gsvp.attr=spl->spl_lattr;
69-
gsvp.len=spl->splitVector.spl_nleft;
7069
gsvp.entries=spl->splitVector.spl_left;
70+
gsvp.len=spl->splitVector.spl_nleft;
71+
gsvp.attr=spl->spl_lattr;
7172
gsvp.isnull=spl->spl_lisnull;
7273

73-
gistunionsubkeyvec(giststate,itvec,&gsvp,attno);
74+
gistunionsubkeyvec(giststate,itvec,&gsvp);
7475

75-
gsvp.attr=spl->spl_rattr;
76-
gsvp.len=spl->splitVector.spl_nright;
7776
gsvp.entries=spl->splitVector.spl_right;
77+
gsvp.len=spl->splitVector.spl_nright;
78+
gsvp.attr=spl->spl_rattr;
7879
gsvp.isnull=spl->spl_risnull;
7980

80-
gistunionsubkeyvec(giststate,itvec,&gsvp,attno);
81+
gistunionsubkeyvec(giststate,itvec,&gsvp);
8182
}
8283

8384
/*
@@ -443,14 +444,14 @@ gistUserPicksplit(Relation r, GistEntryVector *entryvec, int attno, GistSplitVec
443444
*/
444445
if (LenEquiv==0)
445446
{
446-
gistunionsubkey(giststate,itup,v,attno+1);
447+
gistunionsubkey(giststate,itup,v);
447448
}
448449
else
449450
{
450451
cleanupOffsets(sv->spl_left,&sv->spl_nleft,v->spl_equiv,&LenEquiv);
451452
cleanupOffsets(sv->spl_right,&sv->spl_nright,v->spl_equiv,&LenEquiv);
452453

453-
gistunionsubkey(giststate,itup,v,attno+1);
454+
gistunionsubkey(giststate,itup,v);
454455
if (LenEquiv==1)
455456
{
456457
/*
@@ -471,7 +472,7 @@ gistUserPicksplit(Relation r, GistEntryVector *entryvec, int attno, GistSplitVec
471472
* performance, because it's very rarely
472473
*/
473474
v->spl_equiv=NULL;
474-
gistunionsubkey(giststate,itup,v,attno+1);
475+
gistunionsubkey(giststate,itup,v);
475476

476477
return false;
477478
}
@@ -562,7 +563,7 @@ gistSplitByKey(Relation r, Page page, IndexTuple *itup, int len, GISTSTATE *gist
562563
v->splitVector.spl_left[v->splitVector.spl_nleft++]=i;
563564

564565
v->spl_equiv=NULL;
565-
gistunionsubkey(giststate,itup,v,attno);
566+
gistunionsubkey(giststate,itup,v);
566567
}
567568
else
568569
{
@@ -618,7 +619,7 @@ gistSplitByKey(Relation r, Page page, IndexTuple *itup, int len, GISTSTATE *gist
618619

619620
v->splitVector=backupSplit;
620621
/* reunion left and right datums */
621-
gistunionsubkey(giststate,itup,v,attno);
622+
gistunionsubkey(giststate,itup,v);
622623
}
623624
}
624625
}

‎src/backend/access/gist/gistutil.c

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,6 @@
2323
#include"storage/bufmgr.h"
2424
#include"utils/rel.h"
2525

26-
/*
27-
* static *S used for temrorary storage (saves stack and palloc() call)
28-
*/
29-
30-
staticDatumattrS[INDEX_MAX_KEYS];
31-
staticboolisnullS[INDEX_MAX_KEYS];
3226

3327
/*
3428
* Write itup vector to page, has no control of free space.
@@ -150,12 +144,12 @@ gistfillitupvec(IndexTuple *vec, int veclen, int *memlen)
150144
}
151145

152146
/*
153-
* Make unions of keys in IndexTuple vector, return FALSE if itvec contains
154-
* invalid tuple. Resulting Datums aren't compressed.
147+
* Make unions of keys in IndexTuple vector (one union datum per index column).
148+
* Union Datums are returned into the attr/isnull arrays.
149+
* Resulting Datums aren't compressed.
155150
*/
156-
157151
void
158-
gistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,intstartkey,
152+
gistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,
159153
Datum*attr,bool*isnull)
160154
{
161155
inti;
@@ -164,19 +158,12 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
164158

165159
evec= (GistEntryVector*)palloc((len+2)*sizeof(GISTENTRY)+GEVHDRSZ);
166160

167-
for (i=startkey;i<giststate->tupdesc->natts;i++)
161+
for (i=0;i<giststate->tupdesc->natts;i++)
168162
{
169163
intj;
170164

165+
/* Collect non-null datums for this column */
171166
evec->n=0;
172-
if (!isnull[i])
173-
{
174-
gistentryinit(evec->vector[evec->n],attr[i],
175-
NULL,NULL, (OffsetNumber)0,
176-
FALSE);
177-
evec->n++;
178-
}
179-
180167
for (j=0;j<len;j++)
181168
{
182169
Datumdatum;
@@ -194,7 +181,7 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
194181
evec->n++;
195182
}
196183

197-
/* If thistuple vector was all NULLs, the union is NULL */
184+
/* If thiscolumn was all NULLs, the union is NULL */
198185
if (evec->n==0)
199186
{
200187
attr[i]= (Datum)0;
@@ -204,6 +191,7 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
204191
{
205192
if (evec->n==1)
206193
{
194+
/* unionFn may expect at least two inputs */
207195
evec->n=2;
208196
evec->vector[1]=evec->vector[0];
209197
}
@@ -226,11 +214,12 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
226214
IndexTuple
227215
gistunion(Relationr,IndexTuple*itvec,intlen,GISTSTATE*giststate)
228216
{
229-
memset(isnullS, TRUE,sizeof(bool)*giststate->tupdesc->natts);
217+
Datumattr[INDEX_MAX_KEYS];
218+
boolisnull[INDEX_MAX_KEYS];
230219

231-
gistMakeUnionItVec(giststate,itvec,len,0,attrS,isnullS);
220+
gistMakeUnionItVec(giststate,itvec,len,attr,isnull);
232221

233-
returngistFormTuple(giststate,r,attrS,isnullS, false);
222+
returngistFormTuple(giststate,r,attr,isnull, false);
234223
}
235224

236225
/*
@@ -242,12 +231,15 @@ gistMakeUnionKey(GISTSTATE *giststate, int attno,
242231
GISTENTRY*entry2,boolisnull2,
243232
Datum*dst,bool*dstisnull)
244233
{
245-
234+
/* we need a GistEntryVector with room for exactly 2 elements */
235+
union
236+
{
237+
GistEntryVectorgev;
238+
charpadding[2*sizeof(GISTENTRY)+GEVHDRSZ];
239+
}storage;
240+
GistEntryVector*evec=&storage.gev;
246241
intdstsize;
247242

248-
staticcharstorage[2*sizeof(GISTENTRY)+GEVHDRSZ];
249-
GistEntryVector*evec= (GistEntryVector*)storage;
250-
251243
evec->n=2;
252244

253245
if (isnull1&&isnull2)
@@ -323,6 +315,8 @@ gistgetadjusted(Relation r, IndexTuple oldtup, IndexTuple addtup, GISTSTATE *gis
323315
addentries[INDEX_MAX_KEYS];
324316
boololdisnull[INDEX_MAX_KEYS],
325317
addisnull[INDEX_MAX_KEYS];
318+
Datumattr[INDEX_MAX_KEYS];
319+
boolisnull[INDEX_MAX_KEYS];
326320
IndexTuplenewtup=NULL;
327321
inti;
328322

@@ -337,27 +331,28 @@ gistgetadjusted(Relation r, IndexTuple oldtup, IndexTuple addtup, GISTSTATE *gis
337331
gistMakeUnionKey(giststate,i,
338332
oldentries+i,oldisnull[i],
339333
addentries+i,addisnull[i],
340-
attrS+i,isnullS+i);
334+
attr+i,isnull+i);
341335

342336
if (neednew)
343337
/* we already need new key, so we can skip check */
344338
continue;
345339

346-
if (isnullS[i])
340+
if (isnull[i])
347341
/* union of key may be NULL if and only if both keys are NULL */
348342
continue;
349343

350344
if (!addisnull[i])
351345
{
352-
if (oldisnull[i]||gistKeyIsEQ(giststate,i,oldentries[i].key,attrS[i])== false)
346+
if (oldisnull[i]||
347+
!gistKeyIsEQ(giststate,i,oldentries[i].key,attr[i]))
353348
neednew= true;
354349
}
355350
}
356351

357352
if (neednew)
358353
{
359354
/* need to update key */
360-
newtup=gistFormTuple(giststate,r,attrS,isnullS, false);
355+
newtup=gistFormTuple(giststate,r,attr,isnull, false);
361356
newtup->t_tid=oldtup->t_tid;
362357
}
363358

‎src/include/access/gist_private.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ extern void gistdentryinit(GISTSTATE *giststate, int nkey, GISTENTRY *e,
362362
externfloatgistpenalty(GISTSTATE*giststate,intattno,
363363
GISTENTRY*key1,boolisNull1,
364364
GISTENTRY*key2,boolisNull2);
365-
externvoidgistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,intstartkey,
365+
externvoidgistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,
366366
Datum*attr,bool*isnull);
367367
externboolgistKeyIsEQ(GISTSTATE*giststate,intattno,Datuma,Datumb);
368368
externvoidgistDeCompressAtt(GISTSTATE*giststate,Relationr,IndexTupletuple,Pagep,

0 commit comments

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