Re: [HACKERS] [COMMITTERS] pgsql: Fix cardinality estimates for parallel joins.

2017-03-15 Thread Amit Kapila
On Tue, Mar 14, 2017 at 9:59 PM, Robert Haas  wrote:
> On Tue, Jan 17, 2017 at 11:49 AM, Robert Haas  wrote:
>> On Mon, Jan 16, 2017 at 7:23 AM, Amit Kapila  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


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


Re: [HACKERS] [COMMITTERS] pgsql: Fix cardinality estimates for parallel joins.

2017-03-14 Thread Robert Haas
On Tue, Jan 17, 2017 at 11:49 AM, Robert Haas  wrote:
> On Mon, Jan 16, 2017 at 7:23 AM, Amit Kapila  wrote:
>> On Sat, Jan 14, 2017 at 12:07 AM, Robert Haas  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);
>>
>>
>> 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


clamp-parallel-join-est.patch
Description: Binary data

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


Re: [HACKERS] [COMMITTERS] pgsql: Fix cardinality estimates for parallel joins.

2017-01-17 Thread Robert Haas
On Mon, Jan 16, 2017 at 7:23 AM, Amit Kapila  wrote:
> On Sat, Jan 14, 2017 at 12:07 AM, Robert Haas  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);
>
>
> 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


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