Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 35afcca

Browse files
committed
Reset, not recreate, execGrouping.c style hashtables.
This uses the facility added in the preceding commit to fix performance issues caused by rebuilding the hashtable (with its comparator expression being the most expensive bit), after every reset. That's especially important when the comparator is JIT compiled. Bug: #15592 #15486 Reported-By: Jakub Janeček, Dmitry Marakasov Author: Andres Freund Discussion: https://postgr.es/m/15486-05850f065da42931@postgresql.org https://postgr.es/m/20190114180423.ywhdg2iagzvh43we@alap3.anarazel.de Backpatch: 11, where I broke this in bf6c614
1 parent 6455c65 commit 35afcca

File tree

4 files changed

+79
-64
lines changed

4 files changed

+79
-64
lines changed

src/backend/executor/nodeAgg.c

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,7 +1245,7 @@ find_unaggregated_cols_walker(Node *node, Bitmapset **colnos)
12451245
}
12461246

12471247
/*
1248-
* Initialize the hash table(s) to empty.
1248+
* (Re-)initialize the hash table(s) to empty.
12491249
*
12501250
* To implement hashed aggregation, we need a hashtable that stores a
12511251
* representative tuple and an array of AggStatePerGroup structs for each
@@ -1256,9 +1256,9 @@ find_unaggregated_cols_walker(Node *node, Bitmapset **colnos)
12561256
* We have a separate hashtable and associated perhash data structure for each
12571257
* grouping set for which we're doing hashing.
12581258
*
1259-
* The hash tables always live in the hashcontext's per-tuple memory context
1260-
* (there is only one of these for all tables together, since they are all
1261-
* reset at the same time).
1259+
* The contents of the hash tables always live in the hashcontext's per-tuple
1260+
* memory context (there is only one of these for all tables together, since
1261+
* they are all reset at the same time).
12621262
*/
12631263
static void
12641264
build_hash_table(AggState *aggstate)
@@ -1277,17 +1277,21 @@ build_hash_table(AggState *aggstate)
12771277

12781278
Assert(perhash->aggnode->numGroups > 0);
12791279

1280-
perhash->hashtable = BuildTupleHashTable(&aggstate->ss.ps,
1281-
perhash->hashslot->tts_tupleDescriptor,
1282-
perhash->numCols,
1283-
perhash->hashGrpColIdxHash,
1284-
perhash->eqfuncoids,
1285-
perhash->hashfunctions,
1286-
perhash->aggnode->numGroups,
1287-
additionalsize,
1288-
aggstate->hashcontext->ecxt_per_tuple_memory,
1289-
tmpmem,
1290-
DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit));
1280+
if (perhash->hashtable)
1281+
ResetTupleHashTable(perhash->hashtable);
1282+
else
1283+
perhash->hashtable = BuildTupleHashTableExt(&aggstate->ss.ps,
1284+
perhash->hashslot->tts_tupleDescriptor,
1285+
perhash->numCols,
1286+
perhash->hashGrpColIdxHash,
1287+
perhash->eqfuncoids,
1288+
perhash->hashfunctions,
1289+
perhash->aggnode->numGroups,
1290+
additionalsize,
1291+
aggstate->ss.ps.state->es_query_cxt,
1292+
aggstate->hashcontext->ecxt_per_tuple_memory,
1293+
tmpmem,
1294+
DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit));
12911295
}
12921296
}
12931297

src/backend/executor/nodeRecursiveunion.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,18 @@ build_hash_table(RecursiveUnionState *rustate)
3737
Assert(node->numCols > 0);
3838
Assert(node->numGroups > 0);
3939

40-
rustate->hashtable = BuildTupleHashTable(&rustate->ps,
41-
desc,
42-
node->numCols,
43-
node->dupColIdx,
44-
rustate->eqfuncoids,
45-
rustate->hashfunctions,
46-
node->numGroups,
47-
0,
48-
rustate->tableContext,
49-
rustate->tempContext,
50-
false);
40+
rustate->hashtable = BuildTupleHashTableExt(&rustate->ps,
41+
desc,
42+
node->numCols,
43+
node->dupColIdx,
44+
rustate->eqfuncoids,
45+
rustate->hashfunctions,
46+
node->numGroups,
47+
0,
48+
rustate->ps.state->es_query_cxt,
49+
rustate->tableContext,
50+
rustate->tempContext,
51+
false);
5152
}
5253

5354

@@ -322,9 +323,9 @@ ExecReScanRecursiveUnion(RecursiveUnionState *node)
322323
if (node->tableContext)
323324
MemoryContextResetAndDeleteChildren(node->tableContext);
324325

325-
/* And rebuild empty hashtable if needed */
326+
/* Empty hashtable if needed */
326327
if (plan->numCols > 0)
327-
build_hash_table(node);
328+
ResetTupleHashTable(node->hashtable);
328329

329330
/* reset processing state */
330331
node->recursing = false;

src/backend/executor/nodeSetOp.c

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,17 +126,18 @@ build_hash_table(SetOpState *setopstate)
126126
Assert(node->strategy == SETOP_HASHED);
127127
Assert(node->numGroups > 0);
128128

129-
setopstate->hashtable = BuildTupleHashTable(&setopstate->ps,
130-
desc,
131-
node->numCols,
132-
node->dupColIdx,
133-
setopstate->eqfuncoids,
134-
setopstate->hashfunctions,
135-
node->numGroups,
136-
0,
137-
setopstate->tableContext,
138-
econtext->ecxt_per_tuple_memory,
139-
false);
129+
setopstate->hashtable = BuildTupleHashTableExt(&setopstate->ps,
130+
desc,
131+
node->numCols,
132+
node->dupColIdx,
133+
setopstate->eqfuncoids,
134+
setopstate->hashfunctions,
135+
node->numGroups,
136+
0,
137+
setopstate->ps.state->es_query_cxt,
138+
setopstate->tableContext,
139+
econtext->ecxt_per_tuple_memory,
140+
false);
140141
}
141142

142143
/*
@@ -634,7 +635,7 @@ ExecReScanSetOp(SetOpState *node)
634635
/* And rebuild empty hashtable if needed */
635636
if (((SetOp *) node->ps.plan)->strategy == SETOP_HASHED)
636637
{
637-
build_hash_table(node);
638+
ResetTupleHashTable(node->hashtable);
638639
node->table_filled = false;
639640
}
640641

src/backend/executor/nodeSubplan.c

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -481,8 +481,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
481481
Assert(subplan->subLinkType == ANY_SUBLINK);
482482

483483
/*
484-
* If we already had any hash tables, destroy 'em; then create empty hash
485-
* table(s).
484+
* If we already had any hash tables, reset 'em; otherwise create empty
485+
* hash table(s).
486486
*
487487
* If we need to distinguish accurately between FALSE and UNKNOWN (i.e.,
488488
* NULL) results of the IN operation, then we have to store subplan output
@@ -505,17 +505,21 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
505505
if (nbuckets < 1)
506506
nbuckets = 1;
507507

508-
node->hashtable = BuildTupleHashTable(node->parent,
509-
node->descRight,
510-
ncols,
511-
node->keyColIdx,
512-
node->tab_eq_funcoids,
513-
node->tab_hash_funcs,
514-
nbuckets,
515-
0,
516-
node->hashtablecxt,
517-
node->hashtempcxt,
518-
false);
508+
if (node->hashtable)
509+
ResetTupleHashTable(node->hashtable);
510+
else
511+
node->hashtable = BuildTupleHashTableExt(node->parent,
512+
node->descRight,
513+
ncols,
514+
node->keyColIdx,
515+
node->tab_eq_funcoids,
516+
node->tab_hash_funcs,
517+
nbuckets,
518+
0,
519+
node->planstate->state->es_query_cxt,
520+
node->hashtablecxt,
521+
node->hashtempcxt,
522+
false);
519523

520524
if (!subplan->unknownEqFalse)
521525
{
@@ -527,17 +531,22 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext)
527531
if (nbuckets < 1)
528532
nbuckets = 1;
529533
}
530-
node->hashnulls = BuildTupleHashTable(node->parent,
531-
node->descRight,
532-
ncols,
533-
node->keyColIdx,
534-
node->tab_eq_funcoids,
535-
node->tab_hash_funcs,
536-
nbuckets,
537-
0,
538-
node->hashtablecxt,
539-
node->hashtempcxt,
540-
false);
534+
535+
if (node->hashnulls)
536+
ResetTupleHashTable(node->hashtable);
537+
else
538+
node->hashnulls = BuildTupleHashTableExt(node->parent,
539+
node->descRight,
540+
ncols,
541+
node->keyColIdx,
542+
node->tab_eq_funcoids,
543+
node->tab_hash_funcs,
544+
nbuckets,
545+
0,
546+
node->planstate->state->es_query_cxt,
547+
node->hashtablecxt,
548+
node->hashtempcxt,
549+
false);
541550
}
542551

543552
/*

0 commit comments

Comments
 (0)