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

Commit166d534

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 parentc5aad8d commit166d534

File tree

3 files changed

+46
-50
lines changed

3 files changed

+46
-50
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 tuplesthataren'tingroups of equivalent tuples
31+
*Form unions of subkeys afterapage split,ignoring any tuples
32+
* thatare 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
{
@@ -617,7 +618,7 @@ gistSplitByKey(Relation r, Page page, IndexTuple *itup, int len, GISTSTATE *gist
617618

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

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

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,6 @@
2121
#include"storage/lmgr.h"
2222
#include"utils/builtins.h"
2323

24-
/*
25-
* static *S used for temporary storage (saves stack and palloc() call)
26-
*/
27-
28-
staticDatumattrS[INDEX_MAX_KEYS];
29-
staticboolisnullS[INDEX_MAX_KEYS];
3024

3125
/*
3226
* Write itup vector to page, has no control of free space.
@@ -148,12 +142,12 @@ gistfillitupvec(IndexTuple *vec, int veclen, int *memlen)
148142
}
149143

150144
/*
151-
* Make unions of keys in IndexTuple vector.
145+
* Make unions of keys in IndexTuple vector (one union datum per index column).
146+
* Union Datums are returned into the attr/isnull arrays.
152147
* Resulting Datums aren't compressed.
153148
*/
154-
155149
void
156-
gistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,intstartkey,
150+
gistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,
157151
Datum*attr,bool*isnull)
158152
{
159153
inti;
@@ -162,19 +156,12 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
162156

163157
evec= (GistEntryVector*)palloc((len+2)*sizeof(GISTENTRY)+GEVHDRSZ);
164158

165-
for (i=startkey;i<giststate->tupdesc->natts;i++)
159+
for (i=0;i<giststate->tupdesc->natts;i++)
166160
{
167161
intj;
168162

163+
/* Collect non-null datums for this column */
169164
evec->n=0;
170-
if (!isnull[i])
171-
{
172-
gistentryinit(evec->vector[evec->n],attr[i],
173-
NULL,NULL, (OffsetNumber)0,
174-
FALSE);
175-
evec->n++;
176-
}
177-
178165
for (j=0;j<len;j++)
179166
{
180167
Datumdatum;
@@ -192,7 +179,7 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
192179
evec->n++;
193180
}
194181

195-
/* If thistuple vector was all NULLs, the union is NULL */
182+
/* If thiscolumn was all NULLs, the union is NULL */
196183
if (evec->n==0)
197184
{
198185
attr[i]= (Datum)0;
@@ -202,6 +189,7 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
202189
{
203190
if (evec->n==1)
204191
{
192+
/* unionFn may expect at least two inputs */
205193
evec->n=2;
206194
evec->vector[1]=evec->vector[0];
207195
}
@@ -224,11 +212,12 @@ gistMakeUnionItVec(GISTSTATE *giststate, IndexTuple *itvec, int len, int startke
224212
IndexTuple
225213
gistunion(Relationr,IndexTuple*itvec,intlen,GISTSTATE*giststate)
226214
{
227-
memset(isnullS, TRUE,sizeof(bool)*giststate->tupdesc->natts);
215+
Datumattr[INDEX_MAX_KEYS];
216+
boolisnull[INDEX_MAX_KEYS];
228217

229-
gistMakeUnionItVec(giststate,itvec,len,0,attrS,isnullS);
218+
gistMakeUnionItVec(giststate,itvec,len,attr,isnull);
230219

231-
returngistFormTuple(giststate,r,attrS,isnullS, false);
220+
returngistFormTuple(giststate,r,attr,isnull, false);
232221
}
233222

234223
/*
@@ -240,12 +229,15 @@ gistMakeUnionKey(GISTSTATE *giststate, int attno,
240229
GISTENTRY*entry2,boolisnull2,
241230
Datum*dst,bool*dstisnull)
242231
{
243-
232+
/* we need a GistEntryVector with room for exactly 2 elements */
233+
union
234+
{
235+
GistEntryVectorgev;
236+
charpadding[2*sizeof(GISTENTRY)+GEVHDRSZ];
237+
}storage;
238+
GistEntryVector*evec=&storage.gev;
244239
intdstsize;
245240

246-
staticcharstorage[2*sizeof(GISTENTRY)+GEVHDRSZ];
247-
GistEntryVector*evec= (GistEntryVector*)storage;
248-
249241
evec->n=2;
250242

251243
if (isnull1&&isnull2)
@@ -321,6 +313,8 @@ gistgetadjusted(Relation r, IndexTuple oldtup, IndexTuple addtup, GISTSTATE *gis
321313
addentries[INDEX_MAX_KEYS];
322314
boololdisnull[INDEX_MAX_KEYS],
323315
addisnull[INDEX_MAX_KEYS];
316+
Datumattr[INDEX_MAX_KEYS];
317+
boolisnull[INDEX_MAX_KEYS];
324318
IndexTuplenewtup=NULL;
325319
inti;
326320

@@ -335,27 +329,28 @@ gistgetadjusted(Relation r, IndexTuple oldtup, IndexTuple addtup, GISTSTATE *gis
335329
gistMakeUnionKey(giststate,i,
336330
oldentries+i,oldisnull[i],
337331
addentries+i,addisnull[i],
338-
attrS+i,isnullS+i);
332+
attr+i,isnull+i);
339333

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

344-
if (isnullS[i])
338+
if (isnull[i])
345339
/* union of key may be NULL if and only if both keys are NULL */
346340
continue;
347341

348342
if (!addisnull[i])
349343
{
350-
if (oldisnull[i]||gistKeyIsEQ(giststate,i,oldentries[i].key,attrS[i])== false)
344+
if (oldisnull[i]||
345+
!gistKeyIsEQ(giststate,i,oldentries[i].key,attr[i]))
351346
neednew= true;
352347
}
353348
}
354349

355350
if (neednew)
356351
{
357352
/* need to update key */
358-
newtup=gistFormTuple(giststate,r,attrS,isnullS, false);
353+
newtup=gistFormTuple(giststate,r,attr,isnull, false);
359354
newtup->t_tid=oldtup->t_tid;
360355
}
361356

‎src/include/access/gist_private.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,7 @@ extern void gistdentryinit(GISTSTATE *giststate, int nkey, GISTENTRY *e,
500500
externfloatgistpenalty(GISTSTATE*giststate,intattno,
501501
GISTENTRY*key1,boolisNull1,
502502
GISTENTRY*key2,boolisNull2);
503-
externvoidgistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,intstartkey,
503+
externvoidgistMakeUnionItVec(GISTSTATE*giststate,IndexTuple*itvec,intlen,
504504
Datum*attr,bool*isnull);
505505
externboolgistKeyIsEQ(GISTSTATE*giststate,intattno,Datuma,Datumb);
506506
externvoidgistDeCompressAtt(GISTSTATE*giststate,Relationr,IndexTupletuple,Pagep,

0 commit comments

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