Fix costing for disk-based hash aggregation.

Report and suggestions from Richard Guo and Tomas Vondra.

Discussion: https://postgr.es/m/CAMbWs4_W8fYbAn8KxgidAaZHON_Oo08OYn9ze=7remJymLqo5g@mail.gmail.com

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 2a6f44a6274c48453396f5ea17f561485c809f53..4c8c5cfc07af05c0497b2726e663fe420620af94 100644 (file)
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -1728,6 +1728,8 @@ hash_agg_set_limits(double hashentrysize, uint64 input_groups, int used_bits,
    /* if not expected to spill, use all of work_mem */
    if (input_groups * hashentrysize < work_mem * 1024L)
    {
+       if (num_partitions != NULL)
+           *num_partitions = 0;
        *mem_limit = work_mem * 1024L;
        *ngroups_limit = *mem_limit / hashentrysize;
        return;

diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 8cf694b61dc8282f22b1f3ee90367f412a0145e6..9e7e57f118fe989deaabb707343aaef407989b41 100644 (file)
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -2257,6 +2257,7 @@ cost_agg(Path *path, PlannerInfo *root,
     */
    if (aggstrategy == AGG_HASHED || aggstrategy == AGG_MIXED)
    {
+       double  pages;
        double  pages_written = 0.0;
        double  pages_read    = 0.0;
        double  hashentrysize;
@@ -2264,7 +2265,7 @@ cost_agg(Path *path, PlannerInfo *root,
        Size    mem_limit;
        uint64  ngroups_limit;
        int     num_partitions;
-
+       int     depth;
 
        /*
         * Estimate number of batches based on the computed limits. If less
@@ -2279,25 +2280,22 @@ cost_agg(Path *path, PlannerInfo *root,
        nbatches = Max( (numGroups * hashentrysize) / mem_limit,
                        numGroups / ngroups_limit );
 
+       nbatches = Max(ceil(nbatches), 1.0);
+       num_partitions = Max(num_partitions, 2);
+
+       /*
+        * The number of partitions can change at different levels of
+        * recursion; but for the purposes of this calculation assume it stays
+        * constant.
+        */
+       depth = ceil( log(nbatches) / log(num_partitions) );
+
        /*
         * Estimate number of pages read and written. For each level of
         * recursion, a tuple must be written and then later read.
         */
-       if (nbatches > 1.0)
-       {
-           double depth;
-           double pages;
-
-           pages = relation_byte_size(input_tuples, input_width) / BLCKSZ;
-
-           /*
-            * The number of partitions can change at different levels of
-            * recursion; but for the purposes of this calculation assume it
-            * stays constant.
-            */
-           depth = ceil( log(nbatches - 1) / log(num_partitions) );
-           pages_written = pages_read = pages * depth;
-       }
+       pages = relation_byte_size(input_tuples, input_width) / BLCKSZ;
+       pages_written = pages_read = pages * depth;
 
        startup_cost += pages_written * random_page_cost;
        total_cost += pages_written * random_page_cost;