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...

src/backend/access/common/reloptions.c

@@ -1206,7 +1206,7 @@ index_reloptions(RegProcedure amoptions, Datum reloptions, bool validate)
 	/* Can't use OidFunctionCallN because we might get a NULL result */
 	fmgr_info(amoptions, &flinfo);
 
-	InitFunctionCallInfoData(fcinfo, &flinfo, 2, NULL, NULL);
+	InitFunctionCallInfoData(fcinfo, &flinfo, 2, InvalidOid, NULL, NULL);
 
 	fcinfo.arg[0] = reloptions;
 	fcinfo.arg[1] = BoolGetDatum(validate);

src/backend/access/common/scankey.c

@@ -42,11 +42,11 @@ ScanKeyEntryInitialize(ScanKey entry,
 	entry->sk_attno = attributeNumber;
 	entry->sk_strategy = strategy;
 	entry->sk_subtype = subtype;
+	entry->sk_collation = collation;
 	entry->sk_argument = argument;
 	if (RegProcedureIsValid(procedure))
 	{
 		fmgr_info(procedure, &entry->sk_func);
-		entry->sk_func.fn_collation = collation;
 	}
 	else
 	{
@@ -83,9 +83,9 @@ ScanKeyInit(ScanKey entry,
 	entry->sk_attno = attributeNumber;
 	entry->sk_strategy = strategy;
 	entry->sk_subtype = InvalidOid;
+	entry->sk_collation = DEFAULT_COLLATION_OID;
 	entry->sk_argument = argument;
 	fmgr_info(procedure, &entry->sk_func);
-	entry->sk_func.fn_collation = DEFAULT_COLLATION_OID;
 }
 
 /*
@@ -111,7 +111,7 @@ ScanKeyEntryInitializeWithInfo(ScanKey entry,
 	entry->sk_attno = attributeNumber;
 	entry->sk_strategy = strategy;
 	entry->sk_subtype = subtype;
+	entry->sk_collation = collation;
 	entry->sk_argument = argument;
 	fmgr_info_copy(&entry->sk_func, finfo, CurrentMemoryContext);
-	entry->sk_func.fn_collation = collation;
 }

src/backend/access/gin/ginget.c

@@ -55,15 +55,16 @@ callConsistentFn(GinState *ginstate, GinScanKey key)
 	 */
 	key->recheckCurItem = true;
 
-	return DatumGetBool(FunctionCall8(&ginstate->consistentFn[key->attnum - 1],
-									  PointerGetDatum(key->entryRes),
-									  UInt16GetDatum(key->strategy),
-									  key->query,
-									  UInt32GetDatum(key->nuserentries),
-									  PointerGetDatum(key->extra_data),
-									  PointerGetDatum(&key->recheckCurItem),
-									  PointerGetDatum(key->queryValues),
-									  PointerGetDatum(key->queryCategories)));
+	return DatumGetBool(FunctionCall8Coll(&ginstate->consistentFn[key->attnum - 1],
+										  ginstate->compareCollation[key->attnum - 1],
+										  PointerGetDatum(key->entryRes),
+										  UInt16GetDatum(key->strategy),
+										  key->query,
+										  UInt32GetDatum(key->nuserentries),
+										  PointerGetDatum(key->extra_data),
+										  PointerGetDatum(&key->recheckCurItem),
+										  PointerGetDatum(key->queryValues),
+										  PointerGetDatum(key->queryCategories)));
 }
 
 /*
@@ -250,9 +251,10 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack *stack,
 			 * case cmp < 0 => not match and continue scan
 			 *----------
 			 */
-			cmp = DatumGetInt32(FunctionCall4(&btree->ginstate->comparePartialFn[attnum - 1],
-											  scanEntry->queryKey,
-											  idatum,
+			cmp = DatumGetInt32(FunctionCall4Coll(&btree->ginstate->comparePartialFn[attnum - 1],
+												  btree->ginstate->compareCollation[attnum - 1],
+												  scanEntry->queryKey,
+												  idatum,
 										 UInt16GetDatum(scanEntry->strategy),
 									PointerGetDatum(scanEntry->extra_data)));
 
@@ -1175,9 +1177,10 @@ matchPartialInPendingList(GinState *ginstate, Page page,
 		 * case cmp < 0 => not match and continue scan
 		 *----------
 		 */
-		cmp = DatumGetInt32(FunctionCall4(&ginstate->comparePartialFn[entry->attnum - 1],
-										  entry->queryKey,
-										  datum[off - 1],
+		cmp = DatumGetInt32(FunctionCall4Coll(&ginstate->comparePartialFn[entry->attnum - 1],
+											  ginstate->compareCollation[entry->attnum - 1],
+											  entry->queryKey,
+											  datum[off - 1],
 										  UInt16GetDatum(entry->strategy),
 										PointerGetDatum(entry->extra_data)));
 		if (cmp == 0)

src/backend/access/gin/ginutil.c

@@ -63,23 +63,6 @@ initGinState(GinState *state, Relation index)
 		fmgr_info_copy(&(state->compareFn[i]),
 					   index_getprocinfo(index, i + 1, GIN_COMPARE_PROC),
 					   CurrentMemoryContext);
-
-		/*
-		 * If the index column has a specified collation, index_getprocinfo
-		 * will have installed it into the fmgr info, and we should honor it.
-		 * However, we may have a collatable storage type for a noncollatable
-		 * indexed data type (for instance, hstore uses text index entries).
-		 * If there's no index collation then specify default collation in
-		 * case the comparison function needs one.	This is harmless if the
-		 * comparison function doesn't care about collation, so we just do it
-		 * unconditionally.  (We could alternatively call get_typcollation,
-		 * but that seems like expensive overkill --- there aren't going to be
-		 * any cases where a GIN storage type has a nondefault collation.)
-		 */
-		if (!OidIsValid(state->compareFn[i].fn_collation))
-			fmgr_info_set_collation(DEFAULT_COLLATION_OID,
-									&(state->compareFn[i]));
-
 		fmgr_info_copy(&(state->extractValueFn[i]),
 					   index_getprocinfo(index, i + 1, GIN_EXTRACTVALUE_PROC),
 					   CurrentMemoryContext);
@@ -98,18 +81,29 @@ initGinState(GinState *state, Relation index)
 			fmgr_info_copy(&(state->comparePartialFn[i]),
 				   index_getprocinfo(index, i + 1, GIN_COMPARE_PARTIAL_PROC),
 						   CurrentMemoryContext);
-
-			/* As above, install collation spec in case compare fn needs it */
-			if (!OidIsValid(state->comparePartialFn[i].fn_collation))
-				fmgr_info_set_collation(DEFAULT_COLLATION_OID,
-										&(state->comparePartialFn[i]));
-
 			state->canPartialMatch[i] = true;
 		}
 		else
 		{
 			state->canPartialMatch[i] = false;
 		}
+
+		/*
+		 * If the index column has a specified collation, we should honor that
+		 * while doing comparisons.  However, we may have a collatable storage
+		 * type for a noncollatable indexed data type (for instance, hstore
+		 * uses text index entries).  If there's no index collation then
+		 * specify default collation in case the comparison function needs
+		 * collation.  This is harmless if the comparison function doesn't
+		 * care about collation, so we just do it unconditionally.  (We could
+		 * alternatively call get_typcollation, but that seems like expensive
+		 * overkill --- there aren't going to be any cases where a GIN storage
+		 * type has a nondefault collation.)
+		 */
+		if (OidIsValid(index->rd_indcollation[i]))
+			state->compareCollation[i] = index->rd_indcollation[i];
+		else
+			state->compareCollation[i] = DEFAULT_COLLATION_OID;
 	}
 }
 
@@ -298,8 +292,9 @@ ginCompareEntries(GinState *ginstate, OffsetNumber attnum,
 		return 0;
 
 	/* both not null, so safe to call the compareFn */
-	return DatumGetInt32(FunctionCall2(&ginstate->compareFn[attnum - 1],
-									   a, b));
+	return DatumGetInt32(FunctionCall2Coll(&ginstate->compareFn[attnum - 1],
+										   ginstate->compareCollation[attnum - 1],
+										   a, b));
 }
 
 /*
@@ -334,6 +329,7 @@ typedef struct
 typedef struct
 {
 	FmgrInfo   *cmpDatumFunc;
+	Oid			collation;
 	bool		haveDups;
 } cmpEntriesArg;
 
@@ -355,8 +351,9 @@ cmpEntries(const void *a, const void *b, void *arg)
 	else if (bb->isnull)
 		res = -1;				/* not-NULL "<" NULL */
 	else
-		res = DatumGetInt32(FunctionCall2(data->cmpDatumFunc,
-										  aa->datum, bb->datum));
+		res = DatumGetInt32(FunctionCall2Coll(data->cmpDatumFunc,
+											  data->collation,
+											  aa->datum, bb->datum));
 
 	/*
 	 * Detect if we have any duplicates.  If there are equal keys, qsort must
@@ -456,6 +453,7 @@ ginExtractEntries(GinState *ginstate, OffsetNumber attnum,
 		}
 
 		arg.cmpDatumFunc = &ginstate->compareFn[attnum - 1];
+		arg.collation = ginstate->compareCollation[attnum - 1];
 		arg.haveDups = false;
 		qsort_arg(keydata, *nentries, sizeof(keyEntryData),
 				  cmpEntries, (void *) &arg);

src/backend/access/gist/gistget.c

@@ -137,12 +137,13 @@ gistindex_keytest(IndexScanDesc scan,
 			 */
 			recheck = true;
 
-			test = FunctionCall5(&key->sk_func,
-								 PointerGetDatum(&de),
-								 key->sk_argument,
-								 Int32GetDatum(key->sk_strategy),
-								 ObjectIdGetDatum(key->sk_subtype),
-								 PointerGetDatum(&recheck));
+			test = FunctionCall5Coll(&key->sk_func,
+									 key->sk_collation,
+									 PointerGetDatum(&de),
+									 key->sk_argument,
+									 Int32GetDatum(key->sk_strategy),
+									 ObjectIdGetDatum(key->sk_subtype),
+									 PointerGetDatum(&recheck));
 
 			if (!DatumGetBool(test))
 				return false;
@@ -195,11 +196,12 @@ gistindex_keytest(IndexScanDesc scan,
 			 * can't tolerate lossy distance calculations on leaf tuples;
 			 * there is no opportunity to re-sort the tuples afterwards.
 			 */
-			dist = FunctionCall4(&key->sk_func,
-								 PointerGetDatum(&de),
-								 key->sk_argument,
-								 Int32GetDatum(key->sk_strategy),
-								 ObjectIdGetDatum(key->sk_subtype));
+			dist = FunctionCall4Coll(&key->sk_func,
+									 key->sk_collation,
+									 PointerGetDatum(&de),
+									 key->sk_argument,
+									 Int32GetDatum(key->sk_strategy),
+									 ObjectIdGetDatum(key->sk_subtype));
 
 			*distance_p = DatumGetFloat8(dist);
 		}

src/backend/access/gist/gistscan.c

@@ -169,8 +169,7 @@ gistrescan(PG_FUNCTION_ARGS)
 		 * comparisons. The original operator is passed to the Consistent
 		 * function in the form of its strategy number, which is available
 		 * from the sk_strategy field, and its subtype from the sk_subtype
-		 * field.  Also, preserve sk_func.fn_collation which is the input
-		 * collation for the operator.
+		 * field.
 		 *
 		 * Next, if any of keys is a NULL and that key is not marked with
 		 * SK_SEARCHNULL/SK_SEARCHNOTNULL then nothing can be found (ie, we
@@ -181,10 +180,8 @@ gistrescan(PG_FUNCTION_ARGS)
 		for (i = 0; i < scan->numberOfKeys; i++)
 		{
 			ScanKey		skey = scan->keyData + i;
-			Oid			collation = skey->sk_func.fn_collation;
 
 			skey->sk_func = so->giststate->consistentFn[skey->sk_attno - 1];
-			skey->sk_func.fn_collation = collation;
 
 			if (skey->sk_flags & SK_ISNULL)
 			{
@@ -205,16 +202,13 @@ gistrescan(PG_FUNCTION_ARGS)
 		 * all comparisons. The original operator is passed to the Distance
 		 * function in the form of its strategy number, which is available
 		 * from the sk_strategy field, and its subtype from the sk_subtype
-		 * field.  Also, preserve sk_func.fn_collation which is the input
-		 * collation for the operator.
+		 * field.
 		 */
 		for (i = 0; i < scan->numberOfOrderBys; i++)
 		{
 			ScanKey		skey = scan->orderByData + i;
-			Oid			collation = skey->sk_func.fn_collation;
 
 			skey->sk_func = so->giststate->distanceFn[skey->sk_attno - 1];
-			skey->sk_func.fn_collation = collation;
 
 			/* Check we actually have a distance function ... */
 			if (!OidIsValid(skey->sk_func.fn_oid))

src/backend/access/hash/hashutil.c

@@ -56,7 +56,8 @@ _hash_checkqual(IndexScanDesc scan, IndexTuple itup)
 		if (key->sk_flags & SK_ISNULL)
 			return false;
 
-		test = FunctionCall2(&key->sk_func, datum, key->sk_argument);
+		test = FunctionCall2Coll(&key->sk_func, key->sk_collation,
+								 datum, key->sk_argument);
 
 		if (!DatumGetBool(test))
 			return false;

src/backend/access/index/indexam.c

@@ -872,7 +872,6 @@ index_getprocinfo(Relation irel,
 				 procnum, attnum, RelationGetRelationName(irel));
 
 		fmgr_info_cxt(procId, locinfo, irel->rd_indexcxt);
-		fmgr_info_set_collation(irel->rd_indcollation[attnum - 1], locinfo);
 	}
 
 	return locinfo;

src/backend/access/nbtree/nbtinsert.c

@@ -2043,9 +2043,10 @@ _bt_isequal(TupleDesc itupdesc, Page page, OffsetNumber offnum,
 		if (isNull || (scankey->sk_flags & SK_ISNULL))
 			return false;
 
-		result = DatumGetInt32(FunctionCall2(&scankey->sk_func,
-											 datum,
-											 scankey->sk_argument));
+		result = DatumGetInt32(FunctionCall2Coll(&scankey->sk_func,
+												 scankey->sk_collation,
+												 datum,
+												 scankey->sk_argument));
 
 		if (result != 0)
 			return false;

src/backend/access/nbtree/nbtsearch.c

@@ -410,9 +410,10 @@ _bt_compare(Relation rel,
 			 * to flip the sign of the comparison result.  (Unless it's a DESC
 			 * column, in which case we *don't* flip the sign.)
 			 */
-			result = DatumGetInt32(FunctionCall2(&scankey->sk_func,
-												 datum,
-												 scankey->sk_argument));
+			result = DatumGetInt32(FunctionCall2Coll(&scankey->sk_func,
+													 scankey->sk_collation,
+													 datum,
+													 scankey->sk_argument));
 
 			if (!(scankey->sk_flags & SK_BT_DESC))
 				result = -result;
@@ -721,7 +722,7 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
 											   cur->sk_attno,
 											   InvalidStrategy,
 											   cur->sk_subtype,
-											   cur->sk_func.fn_collation,
+											   cur->sk_collation,
 											   procinfo,
 											   cur->sk_argument);
 			}
@@ -742,7 +743,7 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
 									   cur->sk_attno,
 									   InvalidStrategy,
 									   cur->sk_subtype,
-									   cur->sk_func.fn_collation,
+									   cur->sk_collation,
 									   cmp_proc,
 									   cur->sk_argument);
 			}

src/backend/access/nbtree/nbtsort.c

@@ -736,9 +736,11 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
 					}
 					else
 					{
-						compare = DatumGetInt32(FunctionCall2(&entry->sk_func,
-															  attrDatum1,
-															  attrDatum2));
+						compare =
+							DatumGetInt32(FunctionCall2Coll(&entry->sk_func,
+															entry->sk_collation,
+															attrDatum1,
+															attrDatum2));
 
 						if (entry->sk_flags & SK_BT_DESC)
 							compare = -compare;

src/backend/access/nbtree/nbtutils.c

@@ -70,8 +70,7 @@ _bt_mkscankey(Relation rel, IndexTuple itup)
 
 		/*
 		 * We can use the cached (default) support procs since no cross-type
-		 * comparison can be needed.  The cached support proc entries have the
-		 * right collation for the index, too.
+		 * comparison can be needed.
 		 */
 		procinfo = index_getprocinfo(rel, i + 1, BTORDER_PROC);
 		arg = index_getattr(itup, i + 1, itupdesc, &null);
@@ -81,7 +80,7 @@ _bt_mkscankey(Relation rel, IndexTuple itup)
 									   (AttrNumber) (i + 1),
 									   InvalidStrategy,
 									   InvalidOid,
-									   procinfo->fn_collation,
+									   rel->rd_indcollation[i],
 									   procinfo,
 									   arg);
 	}
@@ -120,8 +119,7 @@ _bt_mkscankey_nodata(Relation rel)
 
 		/*
 		 * We can use the cached (default) support procs since no cross-type
-		 * comparison can be needed.  The cached support proc entries have the
-		 * right collation for the index, too.
+		 * comparison can be needed.
 		 */
 		procinfo = index_getprocinfo(rel, i + 1, BTORDER_PROC);
 		flags = SK_ISNULL | (indoption[i] << SK_BT_INDOPTION_SHIFT);
@@ -130,7 +128,7 @@ _bt_mkscankey_nodata(Relation rel)
 									   (AttrNumber) (i + 1),
 									   InvalidStrategy,
 									   InvalidOid,
-									   procinfo->fn_collation,
+									   rel->rd_indcollation[i],
 									   procinfo,
 									   (Datum) 0);
 	}
@@ -604,9 +602,10 @@ _bt_compare_scankey_args(IndexScanDesc scan, ScanKey op,
 	 */
 	if (lefttype == opcintype && righttype == optype)
 	{
-		*result = DatumGetBool(FunctionCall2(&op->sk_func,
-											 leftarg->sk_argument,
-											 rightarg->sk_argument));
+		*result = DatumGetBool(FunctionCall2Coll(&op->sk_func,
+												 op->sk_collation,
+												 leftarg->sk_argument,
+												 rightarg->sk_argument));
 		return true;
 	}
 
@@ -633,9 +632,10 @@ _bt_compare_scankey_args(IndexScanDesc scan, ScanKey op,
 
 		if (RegProcedureIsValid(cmp_proc))
 		{
-			*result = DatumGetBool(OidFunctionCall2(cmp_proc,
-													leftarg->sk_argument,
-													rightarg->sk_argument));
+			*result = DatumGetBool(OidFunctionCall2Coll(cmp_proc,
+														op->sk_collation,
+														leftarg->sk_argument,
+														rightarg->sk_argument));
 			return true;
 		}
 	}
@@ -689,6 +689,10 @@ _bt_fix_scankey_strategy(ScanKey skey, int16 *indoption)
 	 * Likewise, "x IS NOT NULL" is supported.	We treat that as either "less
 	 * than NULL" in a NULLS LAST index, or "greater than NULL" in a NULLS
 	 * FIRST index.
+	 *
+	 * Note: someday we might have to fill in sk_collation from the index
+	 * column's collation.  At the moment this is a non-issue because we'll
+	 * never actually call the comparison operator on a NULL.
 	 */
 	if (skey->sk_flags & SK_ISNULL)
 	{
@@ -703,6 +707,7 @@ _bt_fix_scankey_strategy(ScanKey skey, int16 *indoption)
 		{
 			skey->sk_strategy = BTEqualStrategyNumber;
 			skey->sk_subtype = InvalidOid;
+			skey->sk_collation = InvalidOid;
 		}
 		else if (skey->sk_flags & SK_SEARCHNOTNULL)
 		{
@@ -711,6 +716,7 @@ _bt_fix_scankey_strategy(ScanKey skey, int16 *indoption)
 			else
 				skey->sk_strategy = BTLessStrategyNumber;
 			skey->sk_subtype = InvalidOid;
+			skey->sk_collation = InvalidOid;
 		}
 		else
 		{
@@ -976,7 +982,8 @@ _bt_checkkeys(IndexScanDesc scan,
 			return false;
 		}
 
-		test = FunctionCall2(&key->sk_func, datum, key->sk_argument);
+		test = FunctionCall2Coll(&key->sk_func, key->sk_collation,
+								 datum, key->sk_argument);
 
 		if (!DatumGetBool(test))
 		{
@@ -1099,9 +1106,10 @@ _bt_check_rowcompare(ScanKey skey, IndexTuple tuple, TupleDesc tupdesc,
 		}
 
 		/* Perform the test --- three-way comparison not bool operator */
-		cmpresult = DatumGetInt32(FunctionCall2(&subkey->sk_func,
-												datum,
-												subkey->sk_argument));
+		cmpresult = DatumGetInt32(FunctionCall2Coll(&subkey->sk_func,
+													subkey->sk_collation,
+													datum,
+													subkey->sk_argument));
 
 		if (subkey->sk_flags & SK_BT_DESC)
 			cmpresult = -cmpresult;