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

Commit932ab74

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 parent1618b87 commit932ab74

File tree

3 files changed

+51
-53
lines changed

3 files changed

+51
-53
lines changed

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

Lines changed: 23 additions & 21 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,37 @@ gistunionsubkeyvec(GISTSTATE *giststate, IndexTuple *itvec,
4949
cleanedItVec[cleanedLen++]=itvec[gsvp->entries[i]-1];
5050
}
5151

52-
gistMakeUnionItVec(giststate,cleanedItVec,cleanedLen,startkey,
53-
gsvp->attr,gsvp->isnull);
52+
if (!gistMakeUnionItVec(giststate,cleanedItVec,cleanedLen,
53+
gsvp->attr,gsvp->isnull))
54+
elog(ERROR,"invalid GiST tuple not expected");
5455

5556
pfree(cleanedItVec);
5657
}
5758

5859
/*
59-
* unions subkeys for after user picksplit over attno-1 column
60+
* Recompute unions of subkeys after a page split, ignoring any tuples
61+
* that are marked in spl->spl_equiv[]
6062
*/
6163
staticvoid
62-
gistunionsubkey(GISTSTATE*giststate,IndexTuple*itvec,GistSplitVector*spl,intattno)
64+
gistunionsubkey(GISTSTATE*giststate,IndexTuple*itvec,GistSplitVector*spl)
6365
{
6466
GistSplitUniongsvp;
6567

6668
gsvp.equiv=spl->spl_equiv;
6769

68-
gsvp.attr=spl->spl_lattr;
69-
gsvp.len=spl->splitVector.spl_nleft;
7070
gsvp.entries=spl->splitVector.spl_left;
71+
gsvp.len=spl->splitVector.spl_nleft;
72+
gsvp.attr=spl->spl_lattr;
7173
gsvp.isnull=spl->spl_lisnull;
7274

73-
gistunionsubkeyvec(giststate,itvec,&gsvp,attno);
75+
gistunionsubkeyvec(giststate,itvec,&gsvp);
7476

75-
gsvp.attr=spl->spl_rattr;
76-
gsvp.len=spl->splitVector.spl_nright;
7777
gsvp.entries=spl->splitVector.spl_right;
78+
gsvp.len=spl->splitVector.spl_nright;
79+
gsvp.attr=spl->spl_rattr;
7880
gsvp.isnull=spl->spl_risnull;
7981

80-
gistunionsubkeyvec(giststate,itvec,&gsvp,attno);
82+
gistunionsubkeyvec(giststate,itvec,&gsvp);
8183
}
8284

8385
/*
@@ -440,14 +442,14 @@ gistUserPicksplit(Relation r, GistEntryVector *entryvec, int attno, GistSplitVec
440442
*/
441443
if (LenEquiv==0)
442444
{
443-
gistunionsubkey(giststate,itup,v,attno+1);
445+
gistunionsubkey(giststate,itup,v);
444446
}
445447
else
446448
{
447449
cleanupOffsets(sv->spl_left,&sv->spl_nleft,v->spl_equiv,&LenEquiv);
448450
cleanupOffsets(sv->spl_right,&sv->spl_nright,v->spl_equiv,&LenEquiv);
449451

450-
gistunionsubkey(giststate,itup,v,attno+1);
452+
gistunionsubkey(giststate,itup,v);
451453
if (LenEquiv==1)
452454
{
453455
/*
@@ -468,7 +470,7 @@ gistUserPicksplit(Relation r, GistEntryVector *entryvec, int attno, GistSplitVec
468470
* performance, because it's very rarely
469471
*/
470472
v->spl_equiv=NULL;
471-
gistunionsubkey(giststate,itup,v,attno+1);
473+
gistunionsubkey(giststate,itup,v);
472474

473475
return false;
474476
}
@@ -547,7 +549,7 @@ gistSplitByInvalid(GISTSTATE *giststate, GistSplitVector *v, IndexTuple *itup, i
547549
gsvp.entries=v->splitVector.spl_left;
548550
gsvp.isnull=v->spl_lisnull;
549551

550-
gistunionsubkeyvec(giststate,itup,&gsvp,0);
552+
gistunionsubkeyvec(giststate,itup,&gsvp);
551553
}
552554
}
553555

@@ -619,7 +621,7 @@ gistSplitByKey(Relation r, Page page, IndexTuple *itup, int len, GISTSTATE *gist
619621
v->splitVector.spl_left[v->splitVector.spl_nleft++]=i;
620622

621623
v->spl_equiv=NULL;
622-
gistunionsubkey(giststate,itup,v,attno);
624+
gistunionsubkey(giststate,itup,v);
623625
}
624626
else
625627
{
@@ -675,7 +677,7 @@ gistSplitByKey(Relation r, Page page, IndexTuple *itup, int len, GISTSTATE *gist
675677

676678
v->splitVector=backupSplit;
677679
/* reunion left and right datums */
678-
gistunionsubkey(giststate,itup,v,attno);
680+
gistunionsubkey(giststate,itup,v);
679681
}
680682
}
681683
}

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

Lines changed: 27 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,13 @@ 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.
150+
* Fails and returns FALSE if itvec contains any invalid tuples.
155151
*/
156-
157152
bool
158-
gistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,intstartkey,
153+
gistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,
159154
Datum*attr,bool*isnull)
160155
{
161156
inti;
@@ -164,19 +159,12 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
164159

165160
evec= (GistEntryVector*)palloc((len+2)*sizeof(GISTENTRY)+GEVHDRSZ);
166161

167-
for (i=startkey;i<giststate->tupdesc->natts;i++)
162+
for (i=0;i<giststate->tupdesc->natts;i++)
168163
{
169164
intj;
170165

166+
/* Collect non-null datums for this column */
171167
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-
180168
for (j=0;j<len;j++)
181169
{
182170
Datumdatum;
@@ -198,7 +186,7 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
198186
evec->n++;
199187
}
200188

201-
/* If thistuple vector was all NULLs, the union is NULL */
189+
/* If thiscolumn was all NULLs, the union is NULL */
202190
if (evec->n==0)
203191
{
204192
attr[i]= (Datum)0;
@@ -208,6 +196,7 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
208196
{
209197
if (evec->n==1)
210198
{
199+
/* unionFn may expect at least two inputs */
211200
evec->n=2;
212201
evec->vector[1]=evec->vector[0];
213202
}
@@ -231,12 +220,13 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
231220
IndexTuple
232221
gistunion(Relationr,IndexTuple*itvec,intlen,GISTSTATE*giststate)
233222
{
234-
memset(isnullS, TRUE,sizeof(bool)*giststate->tupdesc->natts);
223+
Datumattr[INDEX_MAX_KEYS];
224+
boolisnull[INDEX_MAX_KEYS];
235225

236-
if (!gistMakeUnionItVec(giststate,itvec,len,0,attrS,isnullS))
226+
if (!gistMakeUnionItVec(giststate,itvec,len,attr,isnull))
237227
returngist_form_invalid_tuple(InvalidBlockNumber);
238228

239-
returngistFormTuple(giststate,r,attrS,isnullS, false);
229+
returngistFormTuple(giststate,r,attr,isnull, false);
240230
}
241231

242232
/*
@@ -248,12 +238,15 @@ gistMakeUnionKey(GISTSTATE *giststate, int attno,
248238
GISTENTRY*entry2,boolisnull2,
249239
Datum*dst,bool*dstisnull)
250240
{
251-
241+
/* we need a GistEntryVector with room for exactly 2 elements */
242+
union
243+
{
244+
GistEntryVectorgev;
245+
charpadding[2*sizeof(GISTENTRY)+GEVHDRSZ];
246+
}storage;
247+
GistEntryVector*evec=&storage.gev;
252248
intdstsize;
253249

254-
staticcharstorage[2*sizeof(GISTENTRY)+GEVHDRSZ];
255-
GistEntryVector*evec= (GistEntryVector*)storage;
256-
257250
evec->n=2;
258251

259252
if (isnull1&&isnull2)
@@ -327,6 +320,8 @@ gistgetadjusted(Relation r, IndexTuple oldtup, IndexTuple addtup, GISTSTATE *gis
327320
addentries[INDEX_MAX_KEYS];
328321
boololdisnull[INDEX_MAX_KEYS],
329322
addisnull[INDEX_MAX_KEYS];
323+
Datumattr[INDEX_MAX_KEYS];
324+
boolisnull[INDEX_MAX_KEYS];
330325
IndexTuplenewtup=NULL;
331326
inti;
332327

@@ -344,27 +339,28 @@ gistgetadjusted(Relation r, IndexTuple oldtup, IndexTuple addtup, GISTSTATE *gis
344339
gistMakeUnionKey(giststate,i,
345340
oldentries+i,oldisnull[i],
346341
addentries+i,addisnull[i],
347-
attrS+i,isnullS+i);
342+
attr+i,isnull+i);
348343

349344
if (neednew)
350345
/* we already need new key, so we can skip check */
351346
continue;
352347

353-
if (isnullS[i])
348+
if (isnull[i])
354349
/* union of key may be NULL if and only if both keys are NULL */
355350
continue;
356351

357352
if (!addisnull[i])
358353
{
359-
if (oldisnull[i]||gistKeyIsEQ(giststate,i,oldentries[i].key,attrS[i])== false)
354+
if (oldisnull[i]||
355+
!gistKeyIsEQ(giststate,i,oldentries[i].key,attr[i]))
360356
neednew= true;
361357
}
362358
}
363359

364360
if (neednew)
365361
{
366362
/* need to update key */
367-
newtup=gistFormTuple(giststate,r,attrS,isnullS, false);
363+
newtup=gistFormTuple(giststate,r,attr,isnull, false);
368364
newtup->t_tid=oldtup->t_tid;
369365
}
370366

‎src/include/access/gist_private.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ extern void gistdentryinit(GISTSTATE *giststate, int nkey, GISTENTRY *e,
314314
externfloatgistpenalty(GISTSTATE*giststate,intattno,
315315
GISTENTRY*key1,boolisNull1,
316316
GISTENTRY*key2,boolisNull2);
317-
externboolgistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,intstartkey,
317+
externboolgistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,
318318
Datum*attr,bool*isnull);
319319
externboolgistKeyIsEQ(GISTSTATE*giststate,intattno,Datuma,Datumb);
320320
externvoidgistDeCompressAtt(GISTSTATE*giststate,Relationr,IndexTupletuple,Pagep,

0 commit comments

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