Skip to content

Commit d64713d

Browse files
tglsfdctglsfdc
committed
Pass collations to functions in FunctionCallInfoData, not FmgrInfo.
Since collation is effectively an argument, not a property of the function, FmgrInfo is really the wrong place for it; and this becomes critical in cases where a cached FmgrInfo is used for varying purposes that might need different collation settings. Fix by passing it in FunctionCallInfoData instead. In particular this allows a clean fix for bug #5970 (record_cmp not working). This requires touching a bit more code than the original method, but nobody ever thought that collations would not be an invasive patch...
1 parent 88543ec commit d64713d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+552
-418
lines changed

‎contrib/btree_gin/btree_gin.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ gin_compare_prefix_##type(PG_FUNCTION_ARGS) \
121121
int32 res, \
122122
cmp; \
123123
\
124-
cmp = DatumGetInt32(DirectFunctionCall2WithCollation( \
124+
cmp = DatumGetInt32(DirectFunctionCall2Coll( \
125125
TypeInfo_##type.typecmp, \
126126
DEFAULT_COLLATION_OID, \
127127
(data->strategy == BTLessStrategyNumber || \

‎contrib/btree_gist/btree_text.c

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,37 +33,55 @@ Datum gbt_text_same(PG_FUNCTION_ARGS);
3333
static bool
3434
gbt_textgt(const void *a, const void *b)
3535
{
36-
return (DatumGetBool(DirectFunctionCall2WithCollation(text_gt, DEFAULT_COLLATION_OID, PointerGetDatum(a), PointerGetDatum(b))));
36+
return DatumGetBool(DirectFunctionCall2Coll(text_gt,
37+
DEFAULT_COLLATION_OID,
38+
PointerGetDatum(a),
39+
PointerGetDatum(b)));
3740
}
3841

3942
static bool
4043
gbt_textge(const void *a, const void *b)
4144
{
42-
return (DatumGetBool(DirectFunctionCall2WithCollation(text_ge, DEFAULT_COLLATION_OID, PointerGetDatum(a), PointerGetDatum(b))));
45+
return DatumGetBool(DirectFunctionCall2Coll(text_ge,
46+
DEFAULT_COLLATION_OID,
47+
PointerGetDatum(a),
48+
PointerGetDatum(b)));
4349
}
4450

4551
static bool
4652
gbt_texteq(const void *a, const void *b)
4753
{
48-
return (DatumGetBool(DirectFunctionCall2WithCollation(texteq, DEFAULT_COLLATION_OID, PointerGetDatum(a), PointerGetDatum(b))));
54+
return DatumGetBool(DirectFunctionCall2Coll(texteq,
55+
DEFAULT_COLLATION_OID,
56+
PointerGetDatum(a),
57+
PointerGetDatum(b)));
4958
}
5059

5160
static bool
5261
gbt_textle(const void *a, const void *b)
5362
{
54-
return (DatumGetBool(DirectFunctionCall2WithCollation(text_le, DEFAULT_COLLATION_OID, PointerGetDatum(a), PointerGetDatum(b))));
63+
return DatumGetBool(DirectFunctionCall2Coll(text_le,
64+
DEFAULT_COLLATION_OID,
65+
PointerGetDatum(a),
66+
PointerGetDatum(b)));
5567
}
5668

5769
static bool
5870
gbt_textlt(const void *a, const void *b)
5971
{
60-
return (DatumGetBool(DirectFunctionCall2WithCollation(text_lt, DEFAULT_COLLATION_OID, PointerGetDatum(a), PointerGetDatum(b))));
72+
return DatumGetBool(DirectFunctionCall2Coll(text_lt,
73+
DEFAULT_COLLATION_OID,
74+
PointerGetDatum(a),
75+
PointerGetDatum(b)));
6176
}
6277

6378
static int32
6479
gbt_textcmp(const bytea *a, const bytea *b)
6580
{
66-
return DatumGetInt32(DirectFunctionCall2WithCollation(bttextcmp, DEFAULT_COLLATION_OID, PointerGetDatum(a), PointerGetDatum(b)));
81+
return DatumGetInt32(DirectFunctionCall2Coll(bttextcmp,
82+
DEFAULT_COLLATION_OID,
83+
PointerGetDatum(a),
84+
PointerGetDatum(b)));
6785
}
6886

6987
static gbtree_vinfo tinfo =

‎src/backend/access/common/reloptions.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1206,7 +1206,7 @@ index_reloptions(RegProcedure amoptions, Datum reloptions, bool validate)
12061206
/* Can't use OidFunctionCallN because we might get a NULL result */
12071207
fmgr_info(amoptions, &flinfo);
12081208

1209-
InitFunctionCallInfoData(fcinfo, &flinfo, 2, NULL, NULL);
1209+
InitFunctionCallInfoData(fcinfo, &flinfo, 2, InvalidOid, NULL, NULL);
12101210

12111211
fcinfo.arg[0] = reloptions;
12121212
fcinfo.arg[1] = BoolGetDatum(validate);

‎src/backend/access/common/scankey.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ ScanKeyEntryInitialize(ScanKey entry,
4242
entry->sk_attno = attributeNumber;
4343
entry->sk_strategy = strategy;
4444
entry->sk_subtype = subtype;
45+
entry->sk_collation = collation;
4546
entry->sk_argument = argument;
4647
if (RegProcedureIsValid(procedure))
4748
{
4849
fmgr_info(procedure, &entry->sk_func);
49-
entry->sk_func.fn_collation = collation;
5050
}
5151
else
5252
{
@@ -83,9 +83,9 @@ ScanKeyInit(ScanKey entry,
8383
entry->sk_attno = attributeNumber;
8484
entry->sk_strategy = strategy;
8585
entry->sk_subtype = InvalidOid;
86+
entry->sk_collation = DEFAULT_COLLATION_OID;
8687
entry->sk_argument = argument;
8788
fmgr_info(procedure, &entry->sk_func);
88-
entry->sk_func.fn_collation = DEFAULT_COLLATION_OID;
8989
}
9090

9191
/*
@@ -111,7 +111,7 @@ ScanKeyEntryInitializeWithInfo(ScanKey entry,
111111
entry->sk_attno = attributeNumber;
112112
entry->sk_strategy = strategy;
113113
entry->sk_subtype = subtype;
114+
entry->sk_collation = collation;
114115
entry->sk_argument = argument;
115116
fmgr_info_copy(&entry->sk_func, finfo, CurrentMemoryContext);
116-
entry->sk_func.fn_collation = collation;
117117
}

‎src/backend/access/gin/ginget.c

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,16 @@ callConsistentFn(GinState *ginstate, GinScanKey key)
5555
*/
5656
key->recheckCurItem = true;
5757

58-
return DatumGetBool(FunctionCall8(&ginstate->consistentFn[key->attnum - 1],
59-
PointerGetDatum(key->entryRes),
60-
UInt16GetDatum(key->strategy),
61-
key->query,
62-
UInt32GetDatum(key->nuserentries),
63-
PointerGetDatum(key->extra_data),
64-
PointerGetDatum(&key->recheckCurItem),
65-
PointerGetDatum(key->queryValues),
66-
PointerGetDatum(key->queryCategories)));
58+
return DatumGetBool(FunctionCall8Coll(&ginstate->consistentFn[key->attnum - 1],
59+
ginstate->compareCollation[key->attnum - 1],
60+
PointerGetDatum(key->entryRes),
61+
UInt16GetDatum(key->strategy),
62+
key->query,
63+
UInt32GetDatum(key->nuserentries),
64+
PointerGetDatum(key->extra_data),
65+
PointerGetDatum(&key->recheckCurItem),
66+
PointerGetDatum(key->queryValues),
67+
PointerGetDatum(key->queryCategories)));
6768
}
6869

6970
/*
@@ -250,9 +251,10 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack *stack,
250251
* case cmp < 0 => not match and continue scan
251252
*----------
252253
*/
253-
cmp = DatumGetInt32(FunctionCall4(&btree->ginstate->comparePartialFn[attnum - 1],
254-
scanEntry->queryKey,
255-
idatum,
254+
cmp = DatumGetInt32(FunctionCall4Coll(&btree->ginstate->comparePartialFn[attnum - 1],
255+
btree->ginstate->compareCollation[attnum - 1],
256+
scanEntry->queryKey,
257+
idatum,
256258
UInt16GetDatum(scanEntry->strategy),
257259
PointerGetDatum(scanEntry->extra_data)));
258260

@@ -1175,9 +1177,10 @@ matchPartialInPendingList(GinState *ginstate, Page page,
11751177
* case cmp < 0 => not match and continue scan
11761178
*----------
11771179
*/
1178-
cmp = DatumGetInt32(FunctionCall4(&ginstate->comparePartialFn[entry->attnum - 1],
1179-
entry->queryKey,
1180-
datum[off - 1],
1180+
cmp = DatumGetInt32(FunctionCall4Coll(&ginstate->comparePartialFn[entry->attnum - 1],
1181+
ginstate->compareCollation[entry->attnum - 1],
1182+
entry->queryKey,
1183+
datum[off - 1],
11811184
UInt16GetDatum(entry->strategy),
11821185
PointerGetDatum(entry->extra_data)));
11831186
if (cmp == 0)

‎src/backend/access/gin/ginutil.c

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -63,23 +63,6 @@ initGinState(GinState *state, Relation index)
6363
fmgr_info_copy(&(state->compareFn[i]),
6464
index_getprocinfo(index, i + 1, GIN_COMPARE_PROC),
6565
CurrentMemoryContext);
66-
67-
/*
68-
* If the index column has a specified collation, index_getprocinfo
69-
* will have installed it into the fmgr info, and we should honor it.
70-
* However, we may have a collatable storage type for a noncollatable
71-
* indexed data type (for instance, hstore uses text index entries).
72-
* If there's no index collation then specify default collation in
73-
* case the comparison function needs one. This is harmless if the
74-
* comparison function doesn't care about collation, so we just do it
75-
* unconditionally. (We could alternatively call get_typcollation,
76-
* but that seems like expensive overkill --- there aren't going to be
77-
* any cases where a GIN storage type has a nondefault collation.)
78-
*/
79-
if (!OidIsValid(state->compareFn[i].fn_collation))
80-
fmgr_info_set_collation(DEFAULT_COLLATION_OID,
81-
&(state->compareFn[i]));
82-
8366
fmgr_info_copy(&(state->extractValueFn[i]),
8467
index_getprocinfo(index, i + 1, GIN_EXTRACTVALUE_PROC),
8568
CurrentMemoryContext);
@@ -98,18 +81,29 @@ initGinState(GinState *state, Relation index)
9881
fmgr_info_copy(&(state->comparePartialFn[i]),
9982
index_getprocinfo(index, i + 1, GIN_COMPARE_PARTIAL_PROC),
10083
CurrentMemoryContext);
101-
102-
/* As above, install collation spec in case compare fn needs it */
103-
if (!OidIsValid(state->comparePartialFn[i].fn_collation))
104-
fmgr_info_set_collation(DEFAULT_COLLATION_OID,
105-
&(state->comparePartialFn[i]));
106-
10784
state->canPartialMatch[i] = true;
10885
}
10986
else
11087
{
11188
state->canPartialMatch[i] = false;
11289
}
90+
91+
/*
92+
* If the index column has a specified collation, we should honor that
93+
* while doing comparisons. However, we may have a collatable storage
94+
* type for a noncollatable indexed data type (for instance, hstore
95+
* uses text index entries). If there's no index collation then
96+
* specify default collation in case the comparison function needs
97+
* collation. This is harmless if the comparison function doesn't
98+
* care about collation, so we just do it unconditionally. (We could
99+
* alternatively call get_typcollation, but that seems like expensive
100+
* overkill --- there aren't going to be any cases where a GIN storage
101+
* type has a nondefault collation.)
102+
*/
103+
if (OidIsValid(index->rd_indcollation[i]))
104+
state->compareCollation[i] = index->rd_indcollation[i];
105+
else
106+
state->compareCollation[i] = DEFAULT_COLLATION_OID;
113107
}
114108
}
115109

@@ -298,8 +292,9 @@ ginCompareEntries(GinState *ginstate, OffsetNumber attnum,
298292
return 0;
299293

300294
/* both not null, so safe to call the compareFn */
301-
return DatumGetInt32(FunctionCall2(&ginstate->compareFn[attnum - 1],
302-
a, b));
295+
return DatumGetInt32(FunctionCall2Coll(&ginstate->compareFn[attnum - 1],
296+
ginstate->compareCollation[attnum - 1],
297+
a, b));
303298
}
304299

305300
/*
@@ -334,6 +329,7 @@ typedef struct
334329
typedef struct
335330
{
336331
FmgrInfo *cmpDatumFunc;
332+
Oid collation;
337333
bool haveDups;
338334
} cmpEntriesArg;
339335

@@ -355,8 +351,9 @@ cmpEntries(const void *a, const void *b, void *arg)
355351
else if (bb->isnull)
356352
res = -1; /* not-NULL "<" NULL */
357353
else
358-
res = DatumGetInt32(FunctionCall2(data->cmpDatumFunc,
359-
aa->datum, bb->datum));
354+
res = DatumGetInt32(FunctionCall2Coll(data->cmpDatumFunc,
355+
data->collation,
356+
aa->datum, bb->datum));
360357

361358
/*
362359
* Detect if we have any duplicates. If there are equal keys, qsort must
@@ -456,6 +453,7 @@ ginExtractEntries(GinState *ginstate, OffsetNumber attnum,
456453
}
457454

458455
arg.cmpDatumFunc = &ginstate->compareFn[attnum - 1];
456+
arg.collation = ginstate->compareCollation[attnum - 1];
459457
arg.haveDups = false;
460458
qsort_arg(keydata, *nentries, sizeof(keyEntryData),
461459
cmpEntries, (void *) &arg);

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

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,13 @@ gistindex_keytest(IndexScanDesc scan,
137137
*/
138138
recheck = true;
139139

140-
test = FunctionCall5(&key->sk_func,
141-
PointerGetDatum(&de),
142-
key->sk_argument,
143-
Int32GetDatum(key->sk_strategy),
144-
ObjectIdGetDatum(key->sk_subtype),
145-
PointerGetDatum(&recheck));
140+
test = FunctionCall5Coll(&key->sk_func,
141+
key->sk_collation,
142+
PointerGetDatum(&de),
143+
key->sk_argument,
144+
Int32GetDatum(key->sk_strategy),
145+
ObjectIdGetDatum(key->sk_subtype),
146+
PointerGetDatum(&recheck));
146147

147148
if (!DatumGetBool(test))
148149
return false;
@@ -195,11 +196,12 @@ gistindex_keytest(IndexScanDesc scan,
195196
* can't tolerate lossy distance calculations on leaf tuples;
196197
* there is no opportunity to re-sort the tuples afterwards.
197198
*/
198-
dist = FunctionCall4(&key->sk_func,
199-
PointerGetDatum(&de),
200-
key->sk_argument,
201-
Int32GetDatum(key->sk_strategy),
202-
ObjectIdGetDatum(key->sk_subtype));
199+
dist = FunctionCall4Coll(&key->sk_func,
200+
key->sk_collation,
201+
PointerGetDatum(&de),
202+
key->sk_argument,
203+
Int32GetDatum(key->sk_strategy),
204+
ObjectIdGetDatum(key->sk_subtype));
203205

204206
*distance_p = DatumGetFloat8(dist);
205207
}

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,7 @@ gistrescan(PG_FUNCTION_ARGS)
169169
* comparisons. The original operator is passed to the Consistent
170170
* function in the form of its strategy number, which is available
171171
* from the sk_strategy field, and its subtype from the sk_subtype
172-
* field. Also, preserve sk_func.fn_collation which is the input
173-
* collation for the operator.
172+
* field.
174173
*
175174
* Next, if any of keys is a NULL and that key is not marked with
176175
* SK_SEARCHNULL/SK_SEARCHNOTNULL then nothing can be found (ie, we
@@ -181,10 +180,8 @@ gistrescan(PG_FUNCTION_ARGS)
181180
for (i = 0; i < scan->numberOfKeys; i++)
182181
{
183182
ScanKey skey = scan->keyData + i;
184-
Oid collation = skey->sk_func.fn_collation;
185183

186184
skey->sk_func = so->giststate->consistentFn[skey->sk_attno - 1];
187-
skey->sk_func.fn_collation = collation;
188185

189186
if (skey->sk_flags & SK_ISNULL)
190187
{
@@ -205,16 +202,13 @@ gistrescan(PG_FUNCTION_ARGS)
205202
* all comparisons. The original operator is passed to the Distance
206203
* function in the form of its strategy number, which is available
207204
* from the sk_strategy field, and its subtype from the sk_subtype
208-
* field. Also, preserve sk_func.fn_collation which is the input
209-
* collation for the operator.
205+
* field.
210206
*/
211207
for (i = 0; i < scan->numberOfOrderBys; i++)
212208
{
213209
ScanKey skey = scan->orderByData + i;
214-
Oid collation = skey->sk_func.fn_collation;
215210

216211
skey->sk_func = so->giststate->distanceFn[skey->sk_attno - 1];
217-
skey->sk_func.fn_collation = collation;
218212

219213
/* Check we actually have a distance function ... */
220214
if (!OidIsValid(skey->sk_func.fn_oid))

‎src/backend/access/hash/hashutil.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ _hash_checkqual(IndexScanDesc scan, IndexTuple itup)
5656
if (key->sk_flags & SK_ISNULL)
5757
return false;
5858

59-
test = FunctionCall2(&key->sk_func, datum, key->sk_argument);
59+
test = FunctionCall2Coll(&key->sk_func, key->sk_collation,
60+
datum, key->sk_argument);
6061

6162
if (!DatumGetBool(test))
6263
return false;

‎src/backend/access/index/indexam.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -872,7 +872,6 @@ index_getprocinfo(Relation irel,
872872
procnum, attnum, RelationGetRelationName(irel));
873873

874874
fmgr_info_cxt(procId, locinfo, irel->rd_indexcxt);
875-
fmgr_info_set_collation(irel->rd_indcollation[attnum - 1], locinfo);
876875
}
877876

878877
return locinfo;

‎src/backend/access/nbtree/nbtinsert.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2043,9 +2043,10 @@ _bt_isequal(TupleDesc itupdesc, Page page, OffsetNumber offnum,
20432043
if (isNull || (scankey->sk_flags & SK_ISNULL))
20442044
return false;
20452045

2046-
result = DatumGetInt32(FunctionCall2(&scankey->sk_func,
2047-
datum,
2048-
scankey->sk_argument));
2046+
result = DatumGetInt32(FunctionCall2Coll(&scankey->sk_func,
2047+
scankey->sk_collation,
2048+
datum,
2049+
scankey->sk_argument));
20492050

20502051
if (result != 0)
20512052
return false;

0 commit comments

Comments
 (0)