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.

Author: tglsfdc

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;
 }

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;
 

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]),

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

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);
 			}
 		}
 	}

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);
 	}

src/backend/commands/seclabel.c

@@ -15,7 +15,6 @@
 #include "catalog/catalog.h"
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
-#include "catalog/pg_collation.h"
 #include "catalog/pg_seclabel.h"
 #include "commands/seclabel.h"
 #include "miscadmin.h"
@@ -166,7 +165,6 @@ GetSecurityLabel(const ObjectAddress *object, const char *provider)
 				Anum_pg_seclabel_provider,
 				BTEqualStrategyNumber, F_TEXTEQ,
 				CStringGetTextDatum(provider));
-	ScanKeyEntryInitializeCollation(&keys[3], DEFAULT_COLLATION_OID);
 
 	pg_seclabel = heap_open(SecLabelRelationId, AccessShareLock);
 
@@ -236,7 +234,6 @@ SetSecurityLabel(const ObjectAddress *object,
 				Anum_pg_seclabel_provider,
 				BTEqualStrategyNumber, F_TEXTEQ,
 				CStringGetTextDatum(provider));
-	ScanKeyEntryInitializeCollation(&keys[3], DEFAULT_COLLATION_OID);
 
 	pg_seclabel = heap_open(SecLabelRelationId, RowExclusiveLock);
 

src/backend/commands/sequence.c

@@ -1455,11 +1455,16 @@ pg_sequence_parameters(PG_FUNCTION_ARGS)
 						RelationGetRelationName(seqrel))));
 
 	tupdesc = CreateTemplateTupleDesc(5, false);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 1, "start_value", INT8OID, -1, 0);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 2, "minimum_value", INT8OID, -1, 0);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 3, "maximum_value", INT8OID, -1, 0);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 4, "increment", INT8OID, -1, 0);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 5, "cycle_option", BOOLOID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 1, "start_value",
+					   INT8OID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 2, "minimum_value",
+					   INT8OID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 3, "maximum_value",
+					   INT8OID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 4, "increment",
+					   INT8OID, -1, 0);
+	TupleDescInitEntry(tupdesc, (AttrNumber) 5, "cycle_option",
+					   BOOLOID, -1, 0);
 
 	BlessTupleDesc(tupdesc);
 

src/backend/executor/nodeIndexscan.c

@@ -829,10 +829,9 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, Index scanrelid,
 								   varattno,	/* attribute number to scan */
 								   op_strategy, /* op's strategy */
 								   op_righttype,		/* strategy subtype */
+								   ((OpExpr *) clause)->inputcollid,	/* collation */
 								   opfuncid,	/* reg proc to use */
 								   scanvalue);	/* constant */
-			ScanKeyEntryInitializeCollation(this_scan_key,
-											((OpExpr *) clause)->inputcollid);
 		}
 		else if (IsA(clause, RowCompareExpr))
 		{
@@ -957,10 +956,9 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, Index scanrelid,
 									   varattno,		/* attribute number */
 									   op_strategy,		/* op's strategy */
 									   op_righttype,	/* strategy subtype */
+									   inputcollation,	/* collation */
 									   opfuncid,		/* reg proc to use */
 									   scanvalue);		/* constant */
-				ScanKeyEntryInitializeCollation(this_sub_key,
-												inputcollation);
 				n_sub_key++;
 			}
 
@@ -1042,10 +1040,9 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, Index scanrelid,
 								   varattno,	/* attribute number to scan */
 								   op_strategy, /* op's strategy */
 								   op_righttype,		/* strategy subtype */
+								   saop->inputcollid,	/* collation */
 								   opfuncid,	/* reg proc to use */
 								   (Datum) 0);	/* constant */
-			ScanKeyEntryInitializeCollation(this_scan_key,
-											saop->inputcollid);
 		}
 		else if (IsA(clause, NullTest))
 		{
@@ -1094,6 +1091,7 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, Index scanrelid,
 								   varattno,	/* attribute number to scan */
 								   InvalidStrategy,		/* no strategy */
 								   InvalidOid,	/* no strategy subtype */
+								   InvalidOid,	/* no collation */
 								   InvalidOid,	/* no reg proc for this */
 								   (Datum) 0);	/* constant */
 		}

src/backend/executor/nodeMergeAppend.c

@@ -134,30 +134,32 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags)
 	{
 		Oid		sortFunction;
 		bool	reverse;
+		int		flags;
 
 		if (!get_compare_function_for_ordering_op(node->sortOperators[i],
 												  &sortFunction, &reverse))
 			elog(ERROR, "operator %u is not a valid ordering operator",
 				 node->sortOperators[i]);
 
+		/* We use btree's conventions for encoding directionality */
+		flags = 0;
+		if (reverse)
+			flags |= SK_BT_DESC;
+		if (node->nullsFirst[i])
+			flags |= SK_BT_NULLS_FIRST;
+
 		/*
 		 * We needn't fill in sk_strategy or sk_subtype since these scankeys
 		 * will never be passed to an index.
 		 */
-		ScanKeyInit(&mergestate->ms_scankeys[i],
-					node->sortColIdx[i],
-					InvalidStrategy,
-					sortFunction,
-					(Datum) 0);
-
-		ScanKeyEntryInitializeCollation(&mergestate->ms_scankeys[i],
-										node->collations[i]);
-
-		/* However, we use btree's conventions for encoding directionality */
-		if (reverse)
-			mergestate->ms_scankeys[i].sk_flags |= SK_BT_DESC;
-		if (node->nullsFirst[i])
-			mergestate->ms_scankeys[i].sk_flags |= SK_BT_NULLS_FIRST;
+		ScanKeyEntryInitialize(&mergestate->ms_scankeys[i],
+							   flags,
+							   node->sortColIdx[i],
+							   InvalidStrategy,
+							   InvalidOid,
+							   node->collations[i],
+							   sortFunction,
+							   (Datum) 0);
 	}
 
 	/*