Обсуждение: Re: [HACKERS] [COMMITTERS] pgsql: Fix cardinality estimates for parallel joins.

On Mon, Jan 16, 2017 at 7:23 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Sat, Jan 14, 2017 at 12:07 AM, Robert Haas <rhaas@postgresql.org> wrote:
>> Fix cardinality estimates for parallel joins.
>>
>
> +       /*
> +        * In the case of a parallel plan, the row count needs to represent
> +        * the number of tuples processed per worker.
> +        */
> +       path->rows = clamp_row_est(path->rows / parallel_divisor);
>     }
>
>     path->startup_cost = startup_cost;
> @@ -2014,6 +1996,10 @@ final_cost_nestloop(PlannerInfo *root, NestPath *path,
>     else
>         path->path.rows = path->path.parent->rows;
>
> +   /* For partial paths, scale row estimate. */
> +   if (path->path.parallel_workers > 0)
> +       path->path.rows /= get_parallel_divisor(&path->path);
>
>
> Isn't it better to call clamp_row_est in join costing functions as we
> are doing in cost_seqscan()?  Is there a reason to keep those
> different?

No, those should probably be changed to match.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Jan 17, 2017 at 11:49 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Jan 16, 2017 at 7:23 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Sat, Jan 14, 2017 at 12:07 AM, Robert Haas <rhaas@postgresql.org> wrote:
>>> Fix cardinality estimates for parallel joins.
>>>
>>
>> +       /*
>> +        * In the case of a parallel plan, the row count needs to represent
>> +        * the number of tuples processed per worker.
>> +        */
>> +       path->rows = clamp_row_est(path->rows / parallel_divisor);
>>     }
>>
>>     path->startup_cost = startup_cost;
>> @@ -2014,6 +1996,10 @@ final_cost_nestloop(PlannerInfo *root, NestPath *path,
>>     else
>>         path->path.rows = path->path.parent->rows;
>>
>> +   /* For partial paths, scale row estimate. */
>> +   if (path->path.parallel_workers > 0)
>> +       path->path.rows /= get_parallel_divisor(&path->path);
>>
>>
>> Isn't it better to call clamp_row_est in join costing functions as we
>> are doing in cost_seqscan()?  Is there a reason to keep those
>> different?
>
> No, those should probably be changed to match.

So I guess that'd look something like this?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Mar 14, 2017 at 9:59 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Jan 17, 2017 at 11:49 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Mon, Jan 16, 2017 at 7:23 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>>
>>>
>>> Isn't it better to call clamp_row_est in join costing functions as we
>>> are doing in cost_seqscan()?  Is there a reason to keep those
>>> different?
>>
>> No, those should probably be changed to match.
>
> So I guess that'd look something like this?
>

Yes, the patch looks good to me.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com