Get rid of old version of BuildTupleHashTable().

It was reasonable to preserve the old API of BuildTupleHashTable()
in the back branches, but in HEAD we should actively discourage use
of that version.  There are no remaining callers in core, so just
get rid of it.  Then rename BuildTupleHashTableExt() back to
BuildTupleHashTable().

While at it, fix up the miserably-poorly-maintained header comment
for BuildTupleHashTable[Ext].  It looks like more than one patch in
this area has had the opinion that updating comments is beneath them.

Discussion: https://postgr.es/m/538343.1734646986@sss.pgh.pa.us

src/backend/executor/execGrouping.c

@@ -135,36 +135,43 @@ execTuplesHashPrepare(int numCols,
 /*
  * Construct an empty TupleHashTable
  *
- *  inputOps: slot ops for input hash values, or NULL if unknown or not fixed
+ *	parent: PlanState node that will own this hash table
- *	numCols, keyColIdx: identify the tuple fields to use as lookup key
+ *	inputDesc: tuple descriptor for input tuples
- *	eqfunctions: equality comparison functions to use
+ *	inputOps: slot ops for input tuples, or NULL if unknown or not fixed
- *	hashfunctions: datatype-specific hashing functions to use
+ *	numCols: number of columns to be compared (length of next 4 arrays)
+ *	keyColIdx: indexes of tuple columns to compare
+ *	eqfuncoids: OIDs of equality comparison functions to use
+ *	hashfunctions: FmgrInfos of datatype-specific hashing functions to use
+ *	collations: collations to use in comparisons
  *	nbuckets: initial estimate of hashtable size
  *	additionalsize: size of data stored in ->additional
  *	metacxt: memory context for long-lived allocation, but not per-entry data
  *	tablecxt: memory context in which to store table entries
  *	tempcxt: short-lived context for evaluation hash and comparison functions
+ *	use_variable_hash_iv: if true, adjust hash IV per-parallel-worker
  *
- * The function arrays may be made with execTuplesHashPrepare().  Note they
+ * The hashfunctions array may be made with execTuplesHashPrepare().  Note they
  * are not cross-type functions, but expect to see the table datatype(s)
  * on both sides.
  *
- * Note that keyColIdx, eqfunctions, and hashfunctions must be allocated in
+ * Note that the keyColIdx, hashfunctions, and collations arrays must be
- * storage that will live as long as the hashtable does.
+ * allocated in storage that will live as long as the hashtable does.
  */
 TupleHashTable
-BuildTupleHashTableExt(PlanState *parent,
+BuildTupleHashTable(PlanState *parent,
-					   TupleDesc inputDesc,
+					TupleDesc inputDesc,
-					   const TupleTableSlotOps *inputOps,
+					const TupleTableSlotOps *inputOps,
-					   int numCols, AttrNumber *keyColIdx,
+					int numCols,
-					   const Oid *eqfuncoids,
+					AttrNumber *keyColIdx,
-					   FmgrInfo *hashfunctions,
+					const Oid *eqfuncoids,
-					   Oid *collations,
+					FmgrInfo *hashfunctions,
-					   long nbuckets, Size additionalsize,
+					Oid *collations,
-					   MemoryContext metacxt,
+					long nbuckets,
-					   MemoryContext tablecxt,
+					Size additionalsize,
-					   MemoryContext tempcxt,
+					MemoryContext metacxt,
-					   bool use_variable_hash_iv)
+					MemoryContext tablecxt,
+					MemoryContext tempcxt,
+					bool use_variable_hash_iv)
 {
 	TupleHashTable hashtable;
 	Size		entrysize = sizeof(TupleHashEntryData) + additionalsize;
@@ -216,14 +223,14 @@ BuildTupleHashTableExt(PlanState *parent,
 													&TTSOpsMinimalTuple);
 
 	/*
-	 * If the old reset interface is used (i.e. BuildTupleHashTable, rather
+	 * If the caller fails to make the metacxt different from the tablecxt,
-	 * than BuildTupleHashTableExt), allowing JIT would lead to the generated
+	 * allowing JIT would lead to the generated functions to a) live longer
-	 * functions to a) live longer than the query b) be re-generated each time
+	 * than the query or b) be re-generated each time the table is being
-	 * the table is being reset. Therefore prevent JIT from being used in that
+	 * reset. Therefore prevent JIT from being used in that case, by not
-	 * case, by not providing a parent node (which prevents accessing the
+	 * providing a parent node (which prevents accessing the JitContext in the
-	 * JitContext in the EState).
+	 * EState).
 	 */
-	allow_jit = metacxt != tablecxt;
+	allow_jit = (metacxt != tablecxt);
 
 	/* build hash ExprState for all columns */
 	hashtable->tab_hash_expr = ExecBuildHash32FromAttrs(inputDesc,
@@ -256,41 +263,9 @@ BuildTupleHashTableExt(PlanState *parent,
 	return hashtable;
 }
 
-/*
- * BuildTupleHashTable is a backwards-compatibility wrapper for
- * BuildTupleHashTableExt(), that allocates the hashtable's metadata in
- * tablecxt. Note that hashtables created this way cannot be reset leak-free
- * with ResetTupleHashTable().
- */
-TupleHashTable
-BuildTupleHashTable(PlanState *parent,
-					TupleDesc inputDesc,
-					int numCols, AttrNumber *keyColIdx,
-					const Oid *eqfuncoids,
-					FmgrInfo *hashfunctions,
-					Oid *collations,
-					long nbuckets, Size additionalsize,
-					MemoryContext tablecxt,
-					MemoryContext tempcxt,
-					bool use_variable_hash_iv)
-{
-	return BuildTupleHashTableExt(parent,
-								  inputDesc,
-								  NULL,
-								  numCols, keyColIdx,
-								  eqfuncoids,
-								  hashfunctions,
-								  collations,
-								  nbuckets, additionalsize,
-								  tablecxt,
-								  tablecxt,
-								  tempcxt,
-								  use_variable_hash_iv);
-}
-
 /*
  * Reset contents of the hashtable to be empty, preserving all the non-content
- * state. Note that the tablecxt passed to BuildTupleHashTableExt() should
+ * state. Note that the tablecxt passed to BuildTupleHashTable() should
  * also be reset, otherwise there will be leaks.
  */
 void

src/backend/executor/nodeAgg.c

@@ -1518,20 +1518,20 @@ build_hash_table(AggState *aggstate, int setno, long nbuckets)
 	 */
 	additionalsize = aggstate->numtrans * sizeof(AggStatePerGroupData);
 
-	perhash->hashtable = BuildTupleHashTableExt(&aggstate->ss.ps,
+	perhash->hashtable = BuildTupleHashTable(&aggstate->ss.ps,
-												perhash->hashslot->tts_tupleDescriptor,
+											 perhash->hashslot->tts_tupleDescriptor,
-												perhash->hashslot->tts_ops,
+											 perhash->hashslot->tts_ops,
-												perhash->numCols,
+											 perhash->numCols,
-												perhash->hashGrpColIdxHash,
+											 perhash->hashGrpColIdxHash,
-												perhash->eqfuncoids,
+											 perhash->eqfuncoids,
-												perhash->hashfunctions,
+											 perhash->hashfunctions,
-												perhash->aggnode->grpCollations,
+											 perhash->aggnode->grpCollations,
-												nbuckets,
+											 nbuckets,
-												additionalsize,
+											 additionalsize,
-												metacxt,
+											 metacxt,
-												hashcxt,
+											 hashcxt,
-												tmpcxt,
+											 tmpcxt,
-												DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit));
+											 DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit));
 }
 
 /*

src/backend/executor/nodeRecursiveunion.c

@@ -39,23 +39,23 @@ build_hash_table(RecursiveUnionState *rustate)
 
 	/*
 	 * If both child plans deliver the same fixed tuple slot type, we can tell
-	 * BuildTupleHashTableExt to expect that slot type as input.  Otherwise,
+	 * BuildTupleHashTable to expect that slot type as input.  Otherwise,
 	 * we'll pass NULL denoting that any slot type is possible.
 	 */
-	rustate->hashtable = BuildTupleHashTableExt(&rustate->ps,
+	rustate->hashtable = BuildTupleHashTable(&rustate->ps,
-												desc,
+											 desc,
-												ExecGetCommonChildSlotOps(&rustate->ps),
+											 ExecGetCommonChildSlotOps(&rustate->ps),
-												node->numCols,
+											 node->numCols,
-												node->dupColIdx,
+											 node->dupColIdx,
-												rustate->eqfuncoids,
+											 rustate->eqfuncoids,
-												rustate->hashfunctions,
+											 rustate->hashfunctions,
-												node->dupCollations,
+											 node->dupCollations,
-												node->numGroups,
+											 node->numGroups,
-												0,
+											 0,
-												rustate->ps.state->es_query_cxt,
+											 rustate->ps.state->es_query_cxt,
-												rustate->tableContext,
+											 rustate->tableContext,
-												rustate->tempContext,
+											 rustate->tempContext,
-												false);
+											 false);
 }
 
 

src/backend/executor/nodeSetOp.c

@@ -92,23 +92,23 @@ build_hash_table(SetOpState *setopstate)
 
 	/*
 	 * If both child plans deliver the same fixed tuple slot type, we can tell
-	 * BuildTupleHashTableExt to expect that slot type as input.  Otherwise,
+	 * BuildTupleHashTable to expect that slot type as input.  Otherwise,
 	 * we'll pass NULL denoting that any slot type is possible.
 	 */
-	setopstate->hashtable = BuildTupleHashTableExt(&setopstate->ps,
+	setopstate->hashtable = BuildTupleHashTable(&setopstate->ps,
-												   desc,
+												desc,
-												   ExecGetCommonChildSlotOps(&setopstate->ps),
+												ExecGetCommonChildSlotOps(&setopstate->ps),
-												   node->numCols,
+												node->numCols,
-												   node->cmpColIdx,
+												node->cmpColIdx,
-												   setopstate->eqfuncoids,
+												setopstate->eqfuncoids,
-												   setopstate->hashfunctions,
+												setopstate->hashfunctions,
-												   node->cmpCollations,
+												node->cmpCollations,
-												   node->numGroups,
+												node->numGroups,
-												   0,
+												0,
-												   setopstate->ps.state->es_query_cxt,
+												setopstate->ps.state->es_query_cxt,
-												   setopstate->tableContext,
+												setopstate->tableContext,
-												   econtext->ecxt_per_tuple_memory,
+												econtext->ecxt_per_tuple_memory,
-												   false);
+												false);
 }
 
 /*

src/backend/executor/nodeSubplan.c

@@ -523,7 +523,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
 	 * Because the input slot for each hash table is always the slot resulting
 	 * from an ExecProject(), we can use TTSOpsVirtual for the input ops. This
 	 * saves a needless fetch inner op step for the hashing ExprState created
-	 * in BuildTupleHashTableExt().
+	 * in BuildTupleHashTable().
 	 */
 	MemoryContextReset(node->hashtablecxt);
 	node->havehashrows = false;
@@ -536,20 +536,20 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
 	if (node->hashtable)
 		ResetTupleHashTable(node->hashtable);
 	else
-		node->hashtable = BuildTupleHashTableExt(node->parent,
+		node->hashtable = BuildTupleHashTable(node->parent,
-												 node->descRight,
+											  node->descRight,
-												 &TTSOpsVirtual,
+											  &TTSOpsVirtual,
-												 ncols,
+											  ncols,
-												 node->keyColIdx,
+											  node->keyColIdx,
-												 node->tab_eq_funcoids,
+											  node->tab_eq_funcoids,
-												 node->tab_hash_funcs,
+											  node->tab_hash_funcs,
-												 node->tab_collations,
+											  node->tab_collations,
-												 nbuckets,
+											  nbuckets,
-												 0,
+											  0,
-												 node->planstate->state->es_query_cxt,
+											  node->planstate->state->es_query_cxt,
-												 node->hashtablecxt,
+											  node->hashtablecxt,
-												 node->hashtempcxt,
+											  node->hashtempcxt,
-												 false);
+											  false);
 
 	if (!subplan->unknownEqFalse)
 	{
@@ -565,20 +565,20 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
 		if (node->hashnulls)
 			ResetTupleHashTable(node->hashnulls);
 		else
-			node->hashnulls = BuildTupleHashTableExt(node->parent,
+			node->hashnulls = BuildTupleHashTable(node->parent,
-													 node->descRight,
+												  node->descRight,
-													 &TTSOpsVirtual,
+												  &TTSOpsVirtual,
-													 ncols,
+												  ncols,
-													 node->keyColIdx,
+												  node->keyColIdx,
-													 node->tab_eq_funcoids,
+												  node->tab_eq_funcoids,
-													 node->tab_hash_funcs,
+												  node->tab_hash_funcs,
-													 node->tab_collations,
+												  node->tab_collations,
-													 nbuckets,
+												  nbuckets,
-													 0,
+												  0,
-													 node->planstate->state->es_query_cxt,
+												  node->planstate->state->es_query_cxt,
-													 node->hashtablecxt,
+												  node->hashtablecxt,
-													 node->hashtempcxt,
+												  node->hashtempcxt,
-													 false);
+												  false);
 	}
 	else
 		node->hashnulls = NULL;

src/include/executor/executor.h

@@ -131,24 +131,18 @@ extern void execTuplesHashPrepare(int numCols,
 								  FmgrInfo **hashFunctions);
 extern TupleHashTable BuildTupleHashTable(PlanState *parent,
 										  TupleDesc inputDesc,
-										  int numCols, AttrNumber *keyColIdx,
+										  const TupleTableSlotOps *inputOps,
+										  int numCols,
+										  AttrNumber *keyColIdx,
 										  const Oid *eqfuncoids,
 										  FmgrInfo *hashfunctions,
 										  Oid *collations,
-										  long nbuckets, Size additionalsize,
+										  long nbuckets,
+										  Size additionalsize,
+										  MemoryContext metacxt,
 										  MemoryContext tablecxt,
-										  MemoryContext tempcxt, bool use_variable_hash_iv);
+										  MemoryContext tempcxt,
-extern TupleHashTable BuildTupleHashTableExt(PlanState *parent,
+										  bool use_variable_hash_iv);
-											 TupleDesc inputDesc,
-											 const TupleTableSlotOps *inputOps,
-											 int numCols, AttrNumber *keyColIdx,
-											 const Oid *eqfuncoids,
-											 FmgrInfo *hashfunctions,
-											 Oid *collations,
-											 long nbuckets, Size additionalsize,
-											 MemoryContext metacxt,
-											 MemoryContext tablecxt,
-											 MemoryContext tempcxt, bool use_variable_hash_iv);
 extern TupleHashEntry LookupTupleHashEntry(TupleHashTable hashtable,
 										   TupleTableSlot *slot,
 										   bool *isnew, uint32 *hash);