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;