Clean up cruft around collation initialization for tupdescs and scankeys.

I found actual bugs in GiST and plpgsql; the rest of this is cosmetic
but meant to decrease the odds of future bugs of omission.

6 files changed, 37 insertions, 24 deletions

diff --git a/src/backend/access/common/scankey.c b/src/backend/access/common/scankey.c
index 41cd36fce92..b632408da47 100644
--- a/src/backend/access/common/scankey.c
+++ b/src/backend/access/common/scankey.c
@@ -15,6 +15,7 @@
 #include "postgres.h"
 
 #include "access/skey.h"
+#include "catalog/pg_collation.h"
 
 
 /*
@@ -33,6 +34,7 @@ ScanKeyEntryInitialize(ScanKey entry,
 					   AttrNumber attributeNumber,
 					   StrategyNumber strategy,
 					   Oid subtype,
+					   Oid collation,
 					   RegProcedure procedure,
 					   Datum argument)
 {
@@ -42,7 +44,10 @@ ScanKeyEntryInitialize(ScanKey entry,
 	entry->sk_subtype = subtype;
 	entry->sk_argument = argument;
 	if (RegProcedureIsValid(procedure))
+	{
 		fmgr_info(procedure, &entry->sk_func);
+		entry->sk_func.fn_collation = collation;
+	}
 	else
 	{
 		Assert(flags & (SK_SEARCHNULL | SK_SEARCHNOTNULL));
@@ -53,12 +58,16 @@ ScanKeyEntryInitialize(ScanKey entry,
 /*
  * ScanKeyInit
  *		Shorthand version of ScanKeyEntryInitialize: flags and subtype
- *		are assumed to be zero (the usual value).
+ *		are assumed to be zero (the usual value), and collation is defaulted.
  *
  * This is the recommended version for hardwired lookups in system catalogs.
  * It cannot handle NULL arguments, unary operators, or nondefault operators,
  * but we need none of those features for most hardwired lookups.
  *
+ * We set collation to DEFAULT_COLLATION_OID always.  This is appropriate
+ * for textual columns in system catalogs, and it will be ignored for
+ * non-textual columns, so it's not worth trying to be more finicky.
+ *
  * Note: CurrentMemoryContext at call should be as long-lived as the ScanKey
  * itself, because that's what will be used for any subsidiary info attached
  * to the ScanKey's FmgrInfo record.
@@ -76,6 +85,7 @@ ScanKeyInit(ScanKey entry,
 	entry->sk_subtype = InvalidOid;
 	entry->sk_argument = argument;
 	fmgr_info(procedure, &entry->sk_func);
+	entry->sk_func.fn_collation = DEFAULT_COLLATION_OID;
 }
 
 /*
@@ -93,6 +103,7 @@ ScanKeyEntryInitializeWithInfo(ScanKey entry,
 							   AttrNumber attributeNumber,
 							   StrategyNumber strategy,
 							   Oid subtype,
+							   Oid collation,
 							   FmgrInfo *finfo,
 							   Datum argument)
 {
@@ -102,17 +113,5 @@ ScanKeyEntryInitializeWithInfo(ScanKey entry,
 	entry->sk_subtype = subtype;
 	entry->sk_argument = argument;
 	fmgr_info_copy(&entry->sk_func, finfo, CurrentMemoryContext);
-}
-
-/*
- * ScanKeyEntryInitializeCollation
- *
- * Initialize the collation of a scan key.  This is just a notational
- * convenience and small abstraction.
- */
-void
-ScanKeyEntryInitializeCollation(ScanKey entry,
-								Oid collation)
-{
 	entry->sk_func.fn_collation = collation;
 }
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index d78b08381e0..c06a0271ca5 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -427,6 +427,10 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
  * TupleDescInitEntry
  *		This function initializes a single attribute structure in
  *		a previously allocated tuple descriptor.
+ *
+ * Note that attcollation is set to the default for the specified datatype.
+ * If a nondefault collation is needed, insert it afterwards using
+ * TupleDescInitEntryCollation.
  */
 void
 TupleDescInitEntry(TupleDesc desc,
@@ -496,8 +500,8 @@ TupleDescInitEntry(TupleDesc desc,
 /*
  * TupleDescInitEntryCollation
  *
- * Fill in the collation for an attribute in a previously initialized
- * tuple descriptor.
+ * Assign a nondefault collation to a previously initialized tuple descriptor
+ * entry.
  */
 void
 TupleDescInitEntryCollation(TupleDesc desc,
@@ -571,9 +575,9 @@ BuildDescForRelation(List *schema)
 
 		TupleDescInitEntry(desc, attnum, attname,
 						   atttypid, atttypmod, attdim);
-		TupleDescInitEntryCollation(desc, attnum, attcollation);
 
 		/* Override TupleDescInitEntry's settings as requested */
+		TupleDescInitEntryCollation(desc, attnum, attcollation);
 		if (entry->storage)
 			desc->attrs[attnum - 1]->attstorage = entry->storage;
 
diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index 23965449df2..9c4473c449b 100644
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -56,6 +56,8 @@ initGinState(GinState *state, Relation index)
 							   origTupdesc->attrs[i]->atttypid,
 							   origTupdesc->attrs[i]->atttypmod,
 							   origTupdesc->attrs[i]->attndims);
+			TupleDescInitEntryCollation(state->tupdesc[i], (AttrNumber) 2,
+										origTupdesc->attrs[i]->attcollation);
 		}
 
 		fmgr_info_copy(&(state->compareFn[i]),
diff --git a/src/backend/access/gist/gistscan.c b/src/backend/access/gist/gistscan.c
index c5d32ef7481..0a125e772d0 100644
--- a/src/backend/access/gist/gistscan.c
+++ b/src/backend/access/gist/gistscan.c
@@ -168,7 +168,8 @@ gistrescan(PG_FUNCTION_ARGS)
 		 * all 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.
+		 * field.  Also, preserve sk_func.fn_collation which is the input
+		 * collation for the operator.
 		 *
 		 * 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
@@ -179,8 +180,10 @@ 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)
 			{
@@ -201,13 +204,16 @@ 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.
+		 * field.  Also, preserve sk_func.fn_collation which is the input
+		 * collation for the operator.
 		 */
 		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))
diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.c
index be8d958352b..cb78a1bae16 100644
--- a/src/backend/access/nbtree/nbtsearch.c
+++ b/src/backend/access/nbtree/nbtsearch.c
@@ -721,10 +721,9 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
 											   cur->sk_attno,
 											   InvalidStrategy,
 											   cur->sk_subtype,
+											   cur->sk_func.fn_collation,
 											   procinfo,
 											   cur->sk_argument);
-				ScanKeyEntryInitializeCollation(scankeys + i,
-												cur->sk_func.fn_collation);
 			}
 			else
 			{
@@ -743,10 +742,9 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
 									   cur->sk_attno,
 									   InvalidStrategy,
 									   cur->sk_subtype,
+									   cur->sk_func.fn_collation,
 									   cmp_proc,
 									   cur->sk_argument);
-				ScanKeyEntryInitializeCollation(scankeys + i,
-												cur->sk_func.fn_collation);
 			}
 		}
 	}
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 7ee7ebeb3cb..add932d9428 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -70,7 +70,8 @@ _bt_mkscankey(Relation rel, IndexTuple itup)
 
 		/*
 		 * We can use the cached (default) support procs since no cross-type
-		 * comparison can be needed.
+		 * comparison can be needed.  The cached support proc entries have
+		 * the right collation for the index, too.
 		 */
 		procinfo = index_getprocinfo(rel, i + 1, BTORDER_PROC);
 		arg = index_getattr(itup, i + 1, itupdesc, &null);
@@ -80,6 +81,7 @@ _bt_mkscankey(Relation rel, IndexTuple itup)
 									   (AttrNumber) (i + 1),
 									   InvalidStrategy,
 									   InvalidOid,
+									   procinfo->fn_collation,
 									   procinfo,
 									   arg);
 	}
@@ -118,7 +120,8 @@ _bt_mkscankey_nodata(Relation rel)
 
 		/*
 		 * We can use the cached (default) support procs since no cross-type
-		 * comparison can be needed.
+		 * comparison can be needed.  The cached support proc entries have
+		 * the right collation for the index, too.
 		 */
 		procinfo = index_getprocinfo(rel, i + 1, BTORDER_PROC);
 		flags = SK_ISNULL | (indoption[i] << SK_BT_INDOPTION_SHIFT);
@@ -127,6 +130,7 @@ _bt_mkscankey_nodata(Relation rel)
 									   (AttrNumber) (i + 1),
 									   InvalidStrategy,
 									   InvalidOid,
+									   procinfo->fn_collation,
 									   procinfo,
 									   (Datum) 0);
 	}