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

Commitdb5f98a

Browse files
committed
Improve BRIN infra, minmax opclass and regression test
The minmax opclass was using the wrong support functions whencross-datatypes queries were run. Instead of trying to fix thepg_amproc definitions (which apparently is not possible), use thealready correct pg_amop entries instead. This requires jumping throughmore hoops (read: extra syscache lookups) to obtain the underlyingfunctions to execute, but it is necessary for correctness.Author: Emre Hasegeli, tweaked by ÁlvaroReview: Andreas KarlssonAlso change BrinOpcInfo to record each stored type's typecache entryinstead of just the OID. Turns out that the full type cache isnecessary in brin_deform_tuple: the original code used the indexedtype's byval and typlen properties to extract the stored tuple, which iscorrect in Minmax; but in other implementations that want to storesomething different, that's wrong. The realization that this is a bugcomes from Emre also, but I did not use his patch.I also adopted Emre's regression test code (with smallish changes),which is more complete.
1 parent7be47c5 commitdb5f98a

File tree

10 files changed

+369
-404
lines changed

10 files changed

+369
-404
lines changed

‎contrib/pageinspect/brinfuncs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ brin_page_items(PG_FUNCTION_ARGS)
181181
column->nstored=opcinfo->oi_nstored;
182182
for (i=0;i<opcinfo->oi_nstored;i++)
183183
{
184-
getTypeOutputInfo(opcinfo->oi_typids[i],&output,&isVarlena);
184+
getTypeOutputInfo(opcinfo->oi_typcache[i]->type_id,&output,&isVarlena);
185185
fmgr_info(output,&column->outputFn[i]);
186186
}
187187

‎doc/src/sgml/brin.sgml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -428,8 +428,8 @@ typedef struct BrinOpcInfo
428428
/* Opaque pointer for the opclass' private use */
429429
void *oi_opaque;
430430

431-
/* TypeIDs of the stored columns */
432-
Oid oi_typids[FLEXIBLE_ARRAY_MEMBER];
431+
/* Typecache entries of the stored columns */
432+
TypeCacheEntry *oi_typcache[FLEXIBLE_ARRAY_MEMBER];
433433
} BrinOpcInfo;
434434
</programlisting>
435435
<structname>BrinOpcInfo</>.<structfield>oi_opaque</> can be used by the

‎src/backend/access/brin/brin_minmax.c

Lines changed: 89 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -15,35 +15,21 @@
1515
#include"access/brin_tuple.h"
1616
#include"access/skey.h"
1717
#include"catalog/pg_type.h"
18+
#include"catalog/pg_amop.h"
1819
#include"utils/datum.h"
1920
#include"utils/lsyscache.h"
21+
#include"utils/rel.h"
2022
#include"utils/syscache.h"
2123

2224

23-
/*
24-
* Procedure numbers must not collide with BRIN_PROCNUM defines in
25-
* brin_internal.h. Note we only need inequality functions.
26-
*/
27-
#defineMINMAX_NUM_PROCNUMS4/* # support procs we need */
28-
#definePROCNUM_LESS11
29-
#definePROCNUM_LESSEQUAL12
30-
#definePROCNUM_GREATEREQUAL13
31-
#definePROCNUM_GREATER14
32-
33-
/*
34-
* Subtract this from procnum to obtain index in MinmaxOpaque arrays
35-
* (Must be equal to minimum of private procnums)
36-
*/
37-
#definePROCNUM_BASE11
38-
3925
typedefstructMinmaxOpaque
4026
{
41-
FmgrInfooperators[MINMAX_NUM_PROCNUMS];
42-
boolinited[MINMAX_NUM_PROCNUMS];
27+
Oidcached_subtype;
28+
FmgrInfostrategy_procinfos[BTMaxStrategyNumber];
4329
}MinmaxOpaque;
4430

45-
staticFmgrInfo*minmax_get_procinfo(BrinDesc*bdesc,uint16attno,
46-
uint16procnum);
31+
staticFmgrInfo*minmax_get_strategy_procinfo(BrinDesc*bdesc,uint16attno,
32+
Oidsubtype,uint16strategynum);
4733

4834

4935
Datum
@@ -53,17 +39,17 @@ brin_minmax_opcinfo(PG_FUNCTION_ARGS)
5339
BrinOpcInfo*result;
5440

5541
/*
56-
* opaque->operators is initialized lazily, as indicated by 'inited' which
57-
*is initialized to all false by palloc0.
42+
* opaque->strategy_procinfos is initialized lazily; here it is set to
43+
*all-uninitialized by palloc0 which sets fn_oid to InvalidOid.
5844
*/
5945

6046
result=palloc0(MAXALIGN(SizeofBrinOpcInfo(2))+
6147
sizeof(MinmaxOpaque));
6248
result->oi_nstored=2;
6349
result->oi_opaque= (MinmaxOpaque*)
6450
MAXALIGN((char*)result+SizeofBrinOpcInfo(2));
65-
result->oi_typids[0]=typoid;
66-
result->oi_typids[1]=typoid;
51+
result->oi_typcache[0]=result->oi_typcache[1]=
52+
lookup_type_cache(typoid,0);
6753

6854
PG_RETURN_POINTER(result);
6955
}
@@ -122,7 +108,8 @@ brin_minmax_add_value(PG_FUNCTION_ARGS)
122108
* and update them accordingly. First check if it's less than the
123109
* existing minimum.
124110
*/
125-
cmpFn=minmax_get_procinfo(bdesc,attno,PROCNUM_LESS);
111+
cmpFn=minmax_get_strategy_procinfo(bdesc,attno,attr->atttypid,
112+
BTLessStrategyNumber);
126113
compar=FunctionCall2Coll(cmpFn,colloid,newval,column->bv_values[0]);
127114
if (DatumGetBool(compar))
128115
{
@@ -135,7 +122,8 @@ brin_minmax_add_value(PG_FUNCTION_ARGS)
135122
/*
136123
* And now compare it to the existing maximum.
137124
*/
138-
cmpFn=minmax_get_procinfo(bdesc,attno,PROCNUM_GREATER);
125+
cmpFn=minmax_get_strategy_procinfo(bdesc,attno,attr->atttypid,
126+
BTGreaterStrategyNumber);
139127
compar=FunctionCall2Coll(cmpFn,colloid,newval,column->bv_values[1]);
140128
if (DatumGetBool(compar))
141129
{
@@ -159,10 +147,12 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
159147
BrinDesc*bdesc= (BrinDesc*)PG_GETARG_POINTER(0);
160148
BrinValues*column= (BrinValues*)PG_GETARG_POINTER(1);
161149
ScanKeykey= (ScanKey)PG_GETARG_POINTER(2);
162-
Oidcolloid=PG_GET_COLLATION();
150+
Oidcolloid=PG_GET_COLLATION(),
151+
subtype;
163152
AttrNumberattno;
164153
Datumvalue;
165154
Datummatches;
155+
FmgrInfo*finfo;
166156

167157
Assert(key->sk_attno==column->bv_attno);
168158

@@ -189,18 +179,16 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
189179
PG_RETURN_BOOL(false);
190180

191181
attno=key->sk_attno;
182+
subtype=key->sk_subtype;
192183
value=key->sk_argument;
193184
switch (key->sk_strategy)
194185
{
195186
caseBTLessStrategyNumber:
196-
matches=FunctionCall2Coll(minmax_get_procinfo(bdesc,attno,
197-
PROCNUM_LESS),
198-
colloid,column->bv_values[0],value);
199-
break;
200187
caseBTLessEqualStrategyNumber:
201-
matches=FunctionCall2Coll(minmax_get_procinfo(bdesc,attno,
202-
PROCNUM_LESSEQUAL),
203-
colloid,column->bv_values[0],value);
188+
finfo=minmax_get_strategy_procinfo(bdesc,attno,subtype,
189+
key->sk_strategy);
190+
matches=FunctionCall2Coll(finfo,colloid,column->bv_values[0],
191+
value);
204192
break;
205193
caseBTEqualStrategyNumber:
206194

@@ -209,25 +197,24 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
209197
* the current page range if the minimum value in the range <=
210198
* scan key, and the maximum value >= scan key.
211199
*/
212-
matches=FunctionCall2Coll(minmax_get_procinfo(bdesc,attno,
213-
PROCNUM_LESSEQUAL),
214-
colloid,column->bv_values[0],value);
200+
finfo=minmax_get_strategy_procinfo(bdesc,attno,subtype,
201+
BTLessEqualStrategyNumber);
202+
matches=FunctionCall2Coll(finfo,colloid,column->bv_values[0],
203+
value);
215204
if (!DatumGetBool(matches))
216205
break;
217206
/* max() >= scankey */
218-
matches=FunctionCall2Coll(minmax_get_procinfo(bdesc,attno,
219-
PROCNUM_GREATEREQUAL),
220-
colloid,column->bv_values[1],value);
207+
finfo=minmax_get_strategy_procinfo(bdesc,attno,subtype,
208+
BTGreaterEqualStrategyNumber);
209+
matches=FunctionCall2Coll(finfo,colloid,column->bv_values[1],
210+
value);
221211
break;
222212
caseBTGreaterEqualStrategyNumber:
223-
matches=FunctionCall2Coll(minmax_get_procinfo(bdesc,attno,
224-
PROCNUM_GREATEREQUAL),
225-
colloid,column->bv_values[1],value);
226-
break;
227213
caseBTGreaterStrategyNumber:
228-
matches=FunctionCall2Coll(minmax_get_procinfo(bdesc,attno,
229-
PROCNUM_GREATER),
230-
colloid,column->bv_values[1],value);
214+
finfo=minmax_get_strategy_procinfo(bdesc,attno,subtype,
215+
key->sk_strategy);
216+
matches=FunctionCall2Coll(finfo,colloid,column->bv_values[1],
217+
value);
231218
break;
232219
default:
233220
/* shouldn't happen */
@@ -252,6 +239,7 @@ brin_minmax_union(PG_FUNCTION_ARGS)
252239
Oidcolloid=PG_GET_COLLATION();
253240
AttrNumberattno;
254241
Form_pg_attributeattr;
242+
FmgrInfo*finfo;
255243
boolneedsadj;
256244

257245
Assert(col_a->bv_attno==col_b->bv_attno);
@@ -284,9 +272,10 @@ brin_minmax_union(PG_FUNCTION_ARGS)
284272
}
285273

286274
/* Adjust minimum, if B's min is less than A's min */
287-
needsadj=FunctionCall2Coll(minmax_get_procinfo(bdesc,attno,
288-
PROCNUM_LESS),
289-
colloid,col_b->bv_values[0],col_a->bv_values[0]);
275+
finfo=minmax_get_strategy_procinfo(bdesc,attno,attr->atttypid,
276+
BTLessStrategyNumber);
277+
needsadj=FunctionCall2Coll(finfo,colloid,col_b->bv_values[0],
278+
col_a->bv_values[0]);
290279
if (needsadj)
291280
{
292281
if (!attr->attbyval)
@@ -296,9 +285,10 @@ brin_minmax_union(PG_FUNCTION_ARGS)
296285
}
297286

298287
/* Adjust maximum, if B's max is greater than A's max */
299-
needsadj=FunctionCall2Coll(minmax_get_procinfo(bdesc,attno,
300-
PROCNUM_GREATER),
301-
colloid,col_b->bv_values[1],col_a->bv_values[1]);
288+
finfo=minmax_get_strategy_procinfo(bdesc,attno,attr->atttypid,
289+
BTGreaterStrategyNumber);
290+
needsadj=FunctionCall2Coll(finfo,colloid,col_b->bv_values[1],
291+
col_a->bv_values[1]);
302292
if (needsadj)
303293
{
304294
if (!attr->attbyval)
@@ -311,27 +301,61 @@ brin_minmax_union(PG_FUNCTION_ARGS)
311301
}
312302

313303
/*
314-
*Returnthe procedurecorresponding tothe givenfunction support number.
304+
*Cache and returnthe procedureforthe givenstrategy.
315305
*/
316-
staticFmgrInfo*
317-
minmax_get_procinfo(BrinDesc*bdesc,uint16attno,uint16procnum)
306+
FmgrInfo*
307+
minmax_get_strategy_procinfo(BrinDesc*bdesc,uint16attno,Oidsubtype,
308+
uint16strategynum)
318309
{
319310
MinmaxOpaque*opaque;
320-
uint16basenum=procnum-PROCNUM_BASE;
311+
312+
Assert(strategynum >=1&&
313+
strategynum <=BTMaxStrategyNumber);
321314

322315
opaque= (MinmaxOpaque*)bdesc->bd_info[attno-1]->oi_opaque;
323316

324317
/*
325-
* We cache these in the opaque struct, to avoid repetitive syscache
326-
* lookups.
318+
* We cache the procedures for the previous subtype in the opaque struct,
319+
* to avoid repetitive syscache lookups. If the subtype changed,
320+
* invalidate all the cached entries.
327321
*/
328-
if (!opaque->inited[basenum])
322+
if (opaque->cached_subtype!=subtype)
323+
{
324+
uint16i;
325+
326+
for (i=1;i <=BTMaxStrategyNumber;i++)
327+
opaque->strategy_procinfos[i-1].fn_oid=InvalidOid;
328+
opaque->cached_subtype=subtype;
329+
}
330+
331+
if (opaque->strategy_procinfos[strategynum-1].fn_oid==InvalidOid)
329332
{
330-
fmgr_info_copy(&opaque->operators[basenum],
331-
index_getprocinfo(bdesc->bd_index,attno,procnum),
332-
bdesc->bd_context);
333-
opaque->inited[basenum]= true;
333+
Form_pg_attributeattr;
334+
HeapTupletuple;
335+
Oidopfamily,
336+
oprid;
337+
boolisNull;
338+
339+
opfamily=bdesc->bd_index->rd_opfamily[attno-1];
340+
attr=bdesc->bd_tupdesc->attrs[attno-1];
341+
tuple=SearchSysCache4(AMOPSTRATEGY,ObjectIdGetDatum(opfamily),
342+
ObjectIdGetDatum(attr->atttypid),
343+
ObjectIdGetDatum(subtype),
344+
Int16GetDatum(strategynum));
345+
346+
if (!HeapTupleIsValid(tuple))
347+
elog(ERROR,"missing operator %d(%u,%u) in opfamily %u",
348+
strategynum,attr->atttypid,subtype,opfamily);
349+
350+
oprid=DatumGetObjectId(SysCacheGetAttr(AMOPSTRATEGY,tuple,
351+
Anum_pg_amop_amopopr,&isNull));
352+
ReleaseSysCache(tuple);
353+
Assert(!isNull&&RegProcedureIsValid(oprid));
354+
355+
fmgr_info_cxt(get_opcode(oprid),
356+
&opaque->strategy_procinfos[strategynum-1],
357+
bdesc->bd_context);
334358
}
335359

336-
return&opaque->operators[basenum];
360+
return&opaque->strategy_procinfos[strategynum-1];
337361
}

‎src/backend/access/brin/brin_tuple.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ brtuple_disk_tupdesc(BrinDesc *brdesc)
6868
{
6969
for (j=0;j<brdesc->bd_info[i]->oi_nstored;j++)
7070
TupleDescInitEntry(tupdesc,attno++,NULL,
71-
brdesc->bd_info[i]->oi_typids[j],
71+
brdesc->bd_info[i]->oi_typcache[j]->type_id,
7272
-1,0);
7373
}
7474

@@ -444,8 +444,8 @@ brin_deform_tuple(BrinDesc *brdesc, BrinTuple *tuple)
444444
for (i=0;i<brdesc->bd_info[keyno]->oi_nstored;i++)
445445
dtup->bt_columns[keyno].bv_values[i]=
446446
datumCopy(values[valueno++],
447-
brdesc->bd_tupdesc->attrs[keyno]->attbyval,
448-
brdesc->bd_tupdesc->attrs[keyno]->attlen);
447+
brdesc->bd_info[keyno]->oi_typcache[i]->typbyval,
448+
brdesc->bd_info[keyno]->oi_typcache[i]->typlen);
449449

450450
dtup->bt_columns[keyno].bv_hasnulls=hasnulls[keyno];
451451
dtup->bt_columns[keyno].bv_allnulls= false;

‎src/include/access/brin_internal.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include"storage/bufpage.h"
1717
#include"storage/off.h"
1818
#include"utils/relcache.h"
19+
#include"utils/typcache.h"
1920

2021

2122
/*
@@ -32,13 +33,13 @@ typedef struct BrinOpcInfo
3233
/* Opaque pointer for the opclass' private use */
3334
void*oi_opaque;
3435

35-
/* TypeIDs of the stored columns */
36-
Oidoi_typids[FLEXIBLE_ARRAY_MEMBER];
36+
/* Typecache entries of the stored columns */
37+
TypeCacheEntry*oi_typcache[FLEXIBLE_ARRAY_MEMBER];
3738
}BrinOpcInfo;
3839

3940
/* the size of a BrinOpcInfo for the given number of columns */
4041
#defineSizeofBrinOpcInfo(ncols) \
41-
(offsetof(BrinOpcInfo,oi_typids) + sizeof(Oid) * ncols)
42+
(offsetof(BrinOpcInfo,oi_typcache) + sizeof(TypeCacheEntry *) * ncols)
4243

4344
typedefstructBrinDesc
4445
{

‎src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/*yyyymmddN */
56-
#defineCATALOG_VERSION_NO201505051
56+
#defineCATALOG_VERSION_NO201505071
5757

5858
#endif

0 commit comments

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