Minor refactor of nodeAgg.c.

  * Separate calculation of hash value from the lookup.
  * Split build_hash_table() into two functions.
  * Change lookup_hash_entry() to return AggStatePerGroup. That's all
    the caller needed, anyway.

These changes are to support the upcoming Disk-based Hash Aggregation
work.

Discussion: https://postgr.es/m/31f5ab871a3ad5a1a91a7a797651f20e77ac7ce3.camel%40j-davis.com

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 85311f2303ab7eeaacc18fbee013a8369c277419..2e9a21bf40006945ff2debe67097ffeeb067188d 100644 (file)
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -263,6 +263,7 @@ static void finalize_partialaggregate(AggState *aggstate,
                                      AggStatePerAgg peragg,
                                      AggStatePerGroup pergroupstate,
                                      Datum *resultVal, bool *resultIsNull);
+static void prepare_hash_slot(AggState *aggstate);
 static void prepare_projection_slot(AggState *aggstate,
                                    TupleTableSlot *slot,
                                    int currentSet);
@@ -272,8 +273,9 @@ static void finalize_aggregates(AggState *aggstate,
 static TupleTableSlot *project_aggregates(AggState *aggstate);
 static Bitmapset *find_unaggregated_cols(AggState *aggstate);
 static bool find_unaggregated_cols_walker(Node *node, Bitmapset **colnos);
-static void build_hash_table(AggState *aggstate);
-static TupleHashEntryData *lookup_hash_entry(AggState *aggstate);
+static void build_hash_tables(AggState *aggstate);
+static void build_hash_table(AggState *aggstate, int setno, long nbuckets);
+static AggStatePerGroup lookup_hash_entry(AggState *aggstate, uint32 hash);
 static void lookup_hash_entries(AggState *aggstate);
 static TupleTableSlot *agg_retrieve_direct(AggState *aggstate);
 static void agg_fill_hash_table(AggState *aggstate);
@@ -1035,6 +1037,32 @@ finalize_partialaggregate(AggState *aggstate,
    MemoryContextSwitchTo(oldContext);
 }
 
+/*
+ * Extract the attributes that make up the grouping key into the
+ * hashslot. This is necessary to compute the hash or perform a lookup.
+ */
+static void
+prepare_hash_slot(AggState *aggstate)
+{
+   TupleTableSlot *inputslot = aggstate->tmpcontext->ecxt_outertuple;
+   AggStatePerHash perhash = &aggstate->perhash[aggstate->current_set];
+   TupleTableSlot *hashslot = perhash->hashslot;
+   int             i;
+
+   /* transfer just the needed columns into hashslot */
+   slot_getsomeattrs(inputslot, perhash->largestGrpColIdx);
+   ExecClearTuple(hashslot);
+
+   for (i = 0; i < perhash->numhashGrpCols; i++)
+   {
+       int         varNumber = perhash->hashGrpColIdxInput[i] - 1;
+
+       hashslot->tts_values[i] = inputslot->tts_values[varNumber];
+       hashslot->tts_isnull[i] = inputslot->tts_isnull[varNumber];
+   }
+   ExecStoreVirtualTuple(hashslot);
+}
+
 /*
  * Prepare to finalize and project based on the specified representative tuple
  * slot and grouping set.
@@ -1249,41 +1277,59 @@ find_unaggregated_cols_walker(Node *node, Bitmapset **colnos)
  * they are all reset at the same time).
  */
 static void
-build_hash_table(AggState *aggstate)
+build_hash_tables(AggState *aggstate)
 {
-   MemoryContext tmpmem = aggstate->tmpcontext->ecxt_per_tuple_memory;
-   Size        additionalsize;
-   int         i;
+   int             setno;
 
-   Assert(aggstate->aggstrategy == AGG_HASHED || aggstate->aggstrategy == AGG_MIXED);
-
-   additionalsize = aggstate->numtrans * sizeof(AggStatePerGroupData);
-
-   for (i = 0; i < aggstate->num_hashes; ++i)
+   for (setno = 0; setno < aggstate->num_hashes; ++setno)
    {
-       AggStatePerHash perhash = &aggstate->perhash[i];
+       AggStatePerHash perhash = &aggstate->perhash[setno];
 
        Assert(perhash->aggnode->numGroups > 0);
 
-       if (perhash->hashtable)
-           ResetTupleHashTable(perhash->hashtable);
-       else
-           perhash->hashtable = BuildTupleHashTableExt(&aggstate->ss.ps,
-                                                       perhash->hashslot->tts_tupleDescriptor,
-                                                       perhash->numCols,
-                                                       perhash->hashGrpColIdxHash,
-                                                       perhash->eqfuncoids,
-                                                       perhash->hashfunctions,
-                                                       perhash->aggnode->grpCollations,
-                                                       perhash->aggnode->numGroups,
-                                                       additionalsize,
-                                                       aggstate->ss.ps.state->es_query_cxt,
-                                                       aggstate->hashcontext->ecxt_per_tuple_memory,
-                                                       tmpmem,
-                                                       DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit));
+       build_hash_table(aggstate, setno, perhash->aggnode->numGroups);
    }
 }
 
+/*
+ * Build a single hashtable for this grouping set.
+ */
+static void
+build_hash_table(AggState *aggstate, int setno, long nbuckets)
+{
+   AggStatePerHash perhash = &aggstate->perhash[setno];
+   MemoryContext   metacxt = aggstate->ss.ps.state->es_query_cxt;
+   MemoryContext   hashcxt = aggstate->hashcontext->ecxt_per_tuple_memory;
+   MemoryContext   tmpcxt  = aggstate->tmpcontext->ecxt_per_tuple_memory;
+   Size            additionalsize;
+
+   Assert(aggstate->aggstrategy == AGG_HASHED ||
+          aggstate->aggstrategy == AGG_MIXED);
+
+   /*
+    * Used to make sure initial hash table allocation does not exceed
+    * work_mem. Note that the estimate does not include space for
+    * pass-by-reference transition data values, nor for the representative
+    * tuple of each group.
+    */
+   additionalsize = aggstate->numtrans * sizeof(AggStatePerGroupData);
+
+   perhash->hashtable = BuildTupleHashTableExt(
+       &aggstate->ss.ps,
+       perhash->hashslot->tts_tupleDescriptor,
+       perhash->numCols,
+       perhash->hashGrpColIdxHash,
+       perhash->eqfuncoids,
+       perhash->hashfunctions,
+       perhash->aggnode->grpCollations,
+       nbuckets,
+       additionalsize,
+       metacxt,
+       hashcxt,
+       tmpcxt,
+       DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit));
+}
+
 /*
  * Compute columns that actually need to be stored in hashtable entries.  The
  * incoming tuples from the child plan node will contain grouping columns,
@@ -1441,33 +1487,20 @@ hash_agg_entry_size(int numAggs, Size tupleWidth, Size transitionSpace)
  * set (which the caller must have selected - note that initialize_aggregate
  * depends on this).
  *
- * When called, CurrentMemoryContext should be the per-query context.
+ * When called, CurrentMemoryContext should be the per-query context. The
+ * already-calculated hash value for the tuple must be specified.
  */
-static TupleHashEntryData *
-lookup_hash_entry(AggState *aggstate)
+static AggStatePerGroup
+lookup_hash_entry(AggState *aggstate, uint32 hash)
 {
-   TupleTableSlot *inputslot = aggstate->tmpcontext->ecxt_outertuple;
    AggStatePerHash perhash = &aggstate->perhash[aggstate->current_set];
    TupleTableSlot *hashslot = perhash->hashslot;
    TupleHashEntryData *entry;
    bool        isnew;
-   int         i;
-
-   /* transfer just the needed columns into hashslot */
-   slot_getsomeattrs(inputslot, perhash->largestGrpColIdx);
-   ExecClearTuple(hashslot);
-
-   for (i = 0; i < perhash->numhashGrpCols; i++)
-   {
-       int         varNumber = perhash->hashGrpColIdxInput[i] - 1;
-
-       hashslot->tts_values[i] = inputslot->tts_values[varNumber];
-       hashslot->tts_isnull[i] = inputslot->tts_isnull[varNumber];
-   }
-   ExecStoreVirtualTuple(hashslot);
 
    /* find or create the hashtable entry using the filtered tuple */
-   entry = LookupTupleHashEntry(perhash->hashtable, hashslot, &isnew);
+   entry = LookupTupleHashEntryHash(perhash->hashtable, hashslot, &isnew,
+                                    hash);
 
    if (isnew)
    {
@@ -1492,7 +1525,7 @@ lookup_hash_entry(AggState *aggstate)
        }
    }
 
-   return entry;
+   return entry->additional;
 }
 
 /*
@@ -1510,8 +1543,13 @@ lookup_hash_entries(AggState *aggstate)
 
    for (setno = 0; setno < numHashes; setno++)
    {
+       AggStatePerHash perhash = &aggstate->perhash[setno];
+       uint32          hash;
+
        select_current_set(aggstate, setno, true);
-       pergroup[setno] = lookup_hash_entry(aggstate)->additional;
+       prepare_hash_slot(aggstate);
+       hash = TupleHashTableHash(perhash->hashtable, perhash->hashslot);
+       pergroup[setno] = lookup_hash_entry(aggstate, hash);
    }
 }
 
@@ -2478,7 +2516,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
        aggstate->hash_pergroup = pergroups;
 
        find_hash_columns(aggstate);
-       build_hash_table(aggstate);
+       build_hash_tables(aggstate);
        aggstate->table_filled = false;
    }
 
@@ -3498,7 +3536,7 @@ ExecReScanAgg(AggState *node)
    {
        ReScanExprContext(node->hashcontext);
        /* Rebuild an empty hash table */
-       build_hash_table(node);
+       build_hash_tables(node);
        node->table_filled = false;
        /* iterator will be reset when the table is filled */
    }