From: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: postgres_fdw: oddity in costing aggregate pushdown paths |
Date: | 2018-12-28 08:28:11 |
Message-ID: | 5C25DE9B.5010000@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
(2018/12/04 17:24), Etsuro Fujita wrote:
> (2018/12/03 20:20), Etsuro Fujita wrote:
>> (2018/11/30 18:51), Etsuro Fujita wrote:
>>> (2018/11/28 13:38), Etsuro Fujita wrote:
>>>> BTW another thing I noticed is this comment on costing aggregate
>>>> pushdown paths using local statistics in estimate_path_cost_size:
>>>>
>>>> * Also, core does not care about costing HAVING expressions and
>>>> * adding that to the costs. So similarly, here too we are not
>>>> * considering remote and local conditions for costing.
>>>>
>>>> I think this was true when aggregate pushdown went in, but isn't anymore
>>>> because of commit 7b6c07547190f056b0464098bb5a2247129d7aa2. So we
>>>> should update estimate_path_cost_size so that it accounts for the
>>>> selectivity and cost of the HAVING expressions as well?
>>>
>>> There seems to be no objections, I updated the patch as such. Attached
>>> is an updated version of the patch.
>>
>> I revised some comments a bit and added the commit message. Attached is
>> an updated patch. If there are no objections, I'll apply this to HEAD only.
>
> Done after fixing typos in the commit message.
I noticed that I forgot to modify the cost for evaluating the PathTarget
for each output row accordingly to this change :(. Attached is a patch
for that.
Best regards,
Etsuro Fujita
Attachment | Content-Type | Size |
---|---|---|
fix-postgres-fdw-grouping-path-cost.patch | text/x-patch | 804 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Masahiko Sawada | 2018-12-28 08:31:44 | Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly |
Previous Message | Fabien COELHO | 2018-12-28 08:18:37 | Re: random() (was Re: New GUC to sample log queries) |