From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: postgres_fdw: oddity in costing aggregate pushdown paths
Date: Fri, 28 Dec 2018 17:28:11 +0900
Message-ID: <5C25DE9B.5010000@lab.ntt.co.jp> (raw)
In-Reply-To: <5C0639B7.1080006@lab.ntt.co.jp>
References: <5BFD3EAD.2060301@lab.ntt.co.jp>
<5BFE1BDE.7010306@lab.ntt.co.jp>
<5C010821.7050102@lab.ntt.co.jp>
<5C051180.7090804@lab.ntt.co.jp>
<5C0639B7.1080006@lab.ntt.co.jp>
(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
Attachments:
[text/x-patch] fix-postgres-fdw-grouping-path-cost.patch (803B, ../5C25DE9B.5010000@lab.ntt.co.jp/2-fix-postgres-fdw-grouping-path-cost.patch)
download | inline diff:
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***************
*** 2937,2943 **** estimate_path_cost_size(PlannerInfo *root,
run_cost = ofpinfo->rel_total_cost - ofpinfo->rel_startup_cost;
run_cost += aggcosts.finalCost * numGroups;
run_cost += cpu_tuple_cost * numGroups;
! run_cost += ptarget->cost.per_tuple * numGroups;
/* Accout for the eval cost of HAVING quals, if any */
if (root->parse->havingQual)
--- 2937,2943 ----
run_cost = ofpinfo->rel_total_cost - ofpinfo->rel_startup_cost;
run_cost += aggcosts.finalCost * numGroups;
run_cost += cpu_tuple_cost * numGroups;
! run_cost += ptarget->cost.per_tuple * rows;
/* Accout for the eval cost of HAVING quals, if any */
if (root->parse->havingQual)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: fujita.etsuro@lab.ntt.co.jp
Subject: Re: postgres_fdw: oddity in costing aggregate pushdown paths
In-Reply-To: <5C25DE9B.5010000@lab.ntt.co.jp>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox