Skip to content

Commit e0a2721

Browse files
tglsfdctglsfdc
committed
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
1 parent f0b9000 commit e0a2721

File tree

6 files changed

+115
-146
lines changed

6 files changed

+115
-146
lines changed

‎src/backend/executor/execGrouping.c

+34-59
Original file line numberDiff line numberDiff line change
@@ -135,36 +135,43 @@ execTuplesHashPrepare(int numCols,
135135
/*
136136
* Construct an empty TupleHashTable
137137
*
138-
* inputOps: slot ops for input hash values, or NULL if unknown or not fixed
139-
* numCols, keyColIdx: identify the tuple fields to use as lookup key
140-
* eqfunctions: equality comparison functions to use
141-
* hashfunctions: datatype-specific hashing functions to use
138+
* parent: PlanState node that will own this hash table
139+
* inputDesc: tuple descriptor for input tuples
140+
* inputOps: slot ops for input tuples, or NULL if unknown or not fixed
141+
* numCols: number of columns to be compared (length of next 4 arrays)
142+
* keyColIdx: indexes of tuple columns to compare
143+
* eqfuncoids: OIDs of equality comparison functions to use
144+
* hashfunctions: FmgrInfos of datatype-specific hashing functions to use
145+
* collations: collations to use in comparisons
142146
* nbuckets: initial estimate of hashtable size
143147
* additionalsize: size of data stored in ->additional
144148
* metacxt: memory context for long-lived allocation, but not per-entry data
145149
* tablecxt: memory context in which to store table entries
146150
* tempcxt: short-lived context for evaluation hash and comparison functions
151+
* use_variable_hash_iv: if true, adjust hash IV per-parallel-worker
147152
*
148-
* The function arrays may be made with execTuplesHashPrepare(). Note they
153+
* The hashfunctions array may be made with execTuplesHashPrepare(). Note they
149154
* are not cross-type functions, but expect to see the table datatype(s)
150155
* on both sides.
151156
*
152-
* Note that keyColIdx, eqfunctions, and hashfunctions must be allocated in
153-
* storage that will live as long as the hashtable does.
157+
* Note that the keyColIdx, hashfunctions, and collations arrays must be
158+
* allocated in storage that will live as long as the hashtable does.
154159
*/
155160
TupleHashTable
156-
BuildTupleHashTableExt(PlanState *parent,
157-
TupleDesc inputDesc,
158-
const TupleTableSlotOps *inputOps,
159-
int numCols, AttrNumber *keyColIdx,
160-
const Oid *eqfuncoids,
161-
FmgrInfo *hashfunctions,
162-
Oid *collations,
163-
long nbuckets, Size additionalsize,
164-
MemoryContext metacxt,
165-
MemoryContext tablecxt,
166-
MemoryContext tempcxt,
167-
bool use_variable_hash_iv)
161+
BuildTupleHashTable(PlanState *parent,
162+
TupleDesc inputDesc,
163+
const TupleTableSlotOps *inputOps,
164+
int numCols,
165+
AttrNumber *keyColIdx,
166+
const Oid *eqfuncoids,
167+
FmgrInfo *hashfunctions,
168+
Oid *collations,
169+
long nbuckets,
170+
Size additionalsize,
171+
MemoryContext metacxt,
172+
MemoryContext tablecxt,
173+
MemoryContext tempcxt,
174+
bool use_variable_hash_iv)
168175
{
169176
TupleHashTable hashtable;
170177
Size entrysize = sizeof(TupleHashEntryData) + additionalsize;
@@ -216,14 +223,14 @@ BuildTupleHashTableExt(PlanState *parent,
216223
&TTSOpsMinimalTuple);
217224

218225
/*
219-
* If the old reset interface is used (i.e. BuildTupleHashTable, rather
220-
* than BuildTupleHashTableExt), allowing JIT would lead to the generated
221-
* functions to a) live longer than the query b) be re-generated each time
222-
* the table is being reset. Therefore prevent JIT from being used in that
223-
* case, by not providing a parent node (which prevents accessing the
224-
* JitContext in the EState).
226+
* If the caller fails to make the metacxt different from the tablecxt,
227+
* allowing JIT would lead to the generated functions to a) live longer
228+
* than the query or b) be re-generated each time the table is being
229+
* reset. Therefore prevent JIT from being used in that case, by not
230+
* providing a parent node (which prevents accessing the JitContext in the
231+
* EState).
225232
*/
226-
allow_jit = metacxt != tablecxt;
233+
allow_jit = (metacxt != tablecxt);
227234

228235
/* build hash ExprState for all columns */
229236
hashtable->tab_hash_expr = ExecBuildHash32FromAttrs(inputDesc,
@@ -256,41 +263,9 @@ BuildTupleHashTableExt(PlanState *parent,
256263
return hashtable;
257264
}
258265

259-
/*
260-
* BuildTupleHashTable is a backwards-compatibility wrapper for
261-
* BuildTupleHashTableExt(), that allocates the hashtable's metadata in
262-
* tablecxt. Note that hashtables created this way cannot be reset leak-free
263-
* with ResetTupleHashTable().
264-
*/
265-
TupleHashTable
266-
BuildTupleHashTable(PlanState *parent,
267-
TupleDesc inputDesc,
268-
int numCols, AttrNumber *keyColIdx,
269-
const Oid *eqfuncoids,
270-
FmgrInfo *hashfunctions,
271-
Oid *collations,
272-
long nbuckets, Size additionalsize,
273-
MemoryContext tablecxt,
274-
MemoryContext tempcxt,
275-
bool use_variable_hash_iv)
276-
{
277-
return BuildTupleHashTableExt(parent,
278-
inputDesc,
279-
NULL,
280-
numCols, keyColIdx,
281-
eqfuncoids,
282-
hashfunctions,
283-
collations,
284-
nbuckets, additionalsize,
285-
tablecxt,
286-
tablecxt,
287-
tempcxt,
288-
use_variable_hash_iv);
289-
}
290-
291266
/*
292267
* Reset contents of the hashtable to be empty, preserving all the non-content
293-
* state. Note that the tablecxt passed to BuildTupleHashTableExt() should
268+
* state. Note that the tablecxt passed to BuildTupleHashTable() should
294269
* also be reset, otherwise there will be leaks.
295270
*/
296271
void

‎src/backend/executor/nodeAgg.c

+14-14
Original file line numberDiff line numberDiff line change
@@ -1518,20 +1518,20 @@ build_hash_table(AggState *aggstate, int setno, long nbuckets)
15181518
*/
15191519
additionalsize = aggstate->numtrans * sizeof(AggStatePerGroupData);
15201520

1521-
perhash->hashtable = BuildTupleHashTableExt(&aggstate->ss.ps,
1522-
perhash->hashslot->tts_tupleDescriptor,
1523-
perhash->hashslot->tts_ops,
1524-
perhash->numCols,
1525-
perhash->hashGrpColIdxHash,
1526-
perhash->eqfuncoids,
1527-
perhash->hashfunctions,
1528-
perhash->aggnode->grpCollations,
1529-
nbuckets,
1530-
additionalsize,
1531-
metacxt,
1532-
hashcxt,
1533-
tmpcxt,
1534-
DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit));
1521+
perhash->hashtable = BuildTupleHashTable(&aggstate->ss.ps,
1522+
perhash->hashslot->tts_tupleDescriptor,
1523+
perhash->hashslot->tts_ops,
1524+
perhash->numCols,
1525+
perhash->hashGrpColIdxHash,
1526+
perhash->eqfuncoids,
1527+
perhash->hashfunctions,
1528+
perhash->aggnode->grpCollations,
1529+
nbuckets,
1530+
additionalsize,
1531+
metacxt,
1532+
hashcxt,
1533+
tmpcxt,
1534+
DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit));
15351535
}
15361536

15371537
/*

‎src/backend/executor/nodeRecursiveunion.c

+15-15
Original file line numberDiff line numberDiff line change
@@ -39,23 +39,23 @@ build_hash_table(RecursiveUnionState *rustate)
3939

4040
/*
4141
* If both child plans deliver the same fixed tuple slot type, we can tell
42-
* BuildTupleHashTableExt to expect that slot type as input. Otherwise,
42+
* BuildTupleHashTable to expect that slot type as input. Otherwise,
4343
* we'll pass NULL denoting that any slot type is possible.
4444
*/
45-
rustate->hashtable = BuildTupleHashTableExt(&rustate->ps,
46-
desc,
47-
ExecGetCommonChildSlotOps(&rustate->ps),
48-
node->numCols,
49-
node->dupColIdx,
50-
rustate->eqfuncoids,
51-
rustate->hashfunctions,
52-
node->dupCollations,
53-
node->numGroups,
54-
0,
55-
rustate->ps.state->es_query_cxt,
56-
rustate->tableContext,
57-
rustate->tempContext,
58-
false);
45+
rustate->hashtable = BuildTupleHashTable(&rustate->ps,
46+
desc,
47+
ExecGetCommonChildSlotOps(&rustate->ps),
48+
node->numCols,
49+
node->dupColIdx,
50+
rustate->eqfuncoids,
51+
rustate->hashfunctions,
52+
node->dupCollations,
53+
node->numGroups,
54+
0,
55+
rustate->ps.state->es_query_cxt,
56+
rustate->tableContext,
57+
rustate->tempContext,
58+
false);
5959
}
6060

6161

‎src/backend/executor/nodeSetOp.c

+15-15
Original file line numberDiff line numberDiff line change
@@ -92,23 +92,23 @@ build_hash_table(SetOpState *setopstate)
9292

9393
/*
9494
* If both child plans deliver the same fixed tuple slot type, we can tell
95-
* BuildTupleHashTableExt to expect that slot type as input. Otherwise,
95+
* BuildTupleHashTable to expect that slot type as input. Otherwise,
9696
* we'll pass NULL denoting that any slot type is possible.
9797
*/
98-
setopstate->hashtable = BuildTupleHashTableExt(&setopstate->ps,
99-
desc,
100-
ExecGetCommonChildSlotOps(&setopstate->ps),
101-
node->numCols,
102-
node->cmpColIdx,
103-
setopstate->eqfuncoids,
104-
setopstate->hashfunctions,
105-
node->cmpCollations,
106-
node->numGroups,
107-
0,
108-
setopstate->ps.state->es_query_cxt,
109-
setopstate->tableContext,
110-
econtext->ecxt_per_tuple_memory,
111-
false);
98+
setopstate->hashtable = BuildTupleHashTable(&setopstate->ps,
99+
desc,
100+
ExecGetCommonChildSlotOps(&setopstate->ps),
101+
node->numCols,
102+
node->cmpColIdx,
103+
setopstate->eqfuncoids,
104+
setopstate->hashfunctions,
105+
node->cmpCollations,
106+
node->numGroups,
107+
0,
108+
setopstate->ps.state->es_query_cxt,
109+
setopstate->tableContext,
110+
econtext->ecxt_per_tuple_memory,
111+
false);
112112
}
113113

114114
/*

‎src/backend/executor/nodeSubplan.c

+29-29
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
523523
* Because the input slot for each hash table is always the slot resulting
524524
* from an ExecProject(), we can use TTSOpsVirtual for the input ops. This
525525
* saves a needless fetch inner op step for the hashing ExprState created
526-
* in BuildTupleHashTableExt().
526+
* in BuildTupleHashTable().
527527
*/
528528
MemoryContextReset(node->hashtablecxt);
529529
node->havehashrows = false;
@@ -536,20 +536,20 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
536536
if (node->hashtable)
537537
ResetTupleHashTable(node->hashtable);
538538
else
539-
node->hashtable = BuildTupleHashTableExt(node->parent,
540-
node->descRight,
541-
&TTSOpsVirtual,
542-
ncols,
543-
node->keyColIdx,
544-
node->tab_eq_funcoids,
545-
node->tab_hash_funcs,
546-
node->tab_collations,
547-
nbuckets,
548-
0,
549-
node->planstate->state->es_query_cxt,
550-
node->hashtablecxt,
551-
node->hashtempcxt,
552-
false);
539+
node->hashtable = BuildTupleHashTable(node->parent,
540+
node->descRight,
541+
&TTSOpsVirtual,
542+
ncols,
543+
node->keyColIdx,
544+
node->tab_eq_funcoids,
545+
node->tab_hash_funcs,
546+
node->tab_collations,
547+
nbuckets,
548+
0,
549+
node->planstate->state->es_query_cxt,
550+
node->hashtablecxt,
551+
node->hashtempcxt,
552+
false);
553553

554554
if (!subplan->unknownEqFalse)
555555
{
@@ -565,20 +565,20 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
565565
if (node->hashnulls)
566566
ResetTupleHashTable(node->hashnulls);
567567
else
568-
node->hashnulls = BuildTupleHashTableExt(node->parent,
569-
node->descRight,
570-
&TTSOpsVirtual,
571-
ncols,
572-
node->keyColIdx,
573-
node->tab_eq_funcoids,
574-
node->tab_hash_funcs,
575-
node->tab_collations,
576-
nbuckets,
577-
0,
578-
node->planstate->state->es_query_cxt,
579-
node->hashtablecxt,
580-
node->hashtempcxt,
581-
false);
568+
node->hashnulls = BuildTupleHashTable(node->parent,
569+
node->descRight,
570+
&TTSOpsVirtual,
571+
ncols,
572+
node->keyColIdx,
573+
node->tab_eq_funcoids,
574+
node->tab_hash_funcs,
575+
node->tab_collations,
576+
nbuckets,
577+
0,
578+
node->planstate->state->es_query_cxt,
579+
node->hashtablecxt,
580+
node->hashtempcxt,
581+
false);
582582
}
583583
else
584584
node->hashnulls = NULL;

‎src/include/executor/executor.h

+8-14
Original file line numberDiff line numberDiff line change
@@ -131,24 +131,18 @@ extern void execTuplesHashPrepare(int numCols,
131131
FmgrInfo **hashFunctions);
132132
extern TupleHashTable BuildTupleHashTable(PlanState *parent,
133133
TupleDesc inputDesc,
134-
int numCols, AttrNumber *keyColIdx,
134+
const TupleTableSlotOps *inputOps,
135+
int numCols,
136+
AttrNumber *keyColIdx,
135137
const Oid *eqfuncoids,
136138
FmgrInfo *hashfunctions,
137139
Oid *collations,
138-
long nbuckets, Size additionalsize,
140+
long nbuckets,
141+
Size additionalsize,
142+
MemoryContext metacxt,
139143
MemoryContext tablecxt,
140-
MemoryContext tempcxt, bool use_variable_hash_iv);
141-
extern TupleHashTable BuildTupleHashTableExt(PlanState *parent,
142-
TupleDesc inputDesc,
143-
const TupleTableSlotOps *inputOps,
144-
int numCols, AttrNumber *keyColIdx,
145-
const Oid *eqfuncoids,
146-
FmgrInfo *hashfunctions,
147-
Oid *collations,
148-
long nbuckets, Size additionalsize,
149-
MemoryContext metacxt,
150-
MemoryContext tablecxt,
151-
MemoryContext tempcxt, bool use_variable_hash_iv);
144+
MemoryContext tempcxt,
145+
bool use_variable_hash_iv);
152146
extern TupleHashEntry LookupTupleHashEntry(TupleHashTable hashtable,
153147
TupleTableSlot *slot,
154148
bool *isnew, uint32 *hash);

0 commit comments

Comments
 (0)