(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
*** 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)
