Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote:

> As mentioned in the near thread, I think there is another oversight in
> the cost estimation for aggregate pushdown paths in postgres_fdw, IIUC.
>  When costing an aggregate pushdown path using local statistics, we
> re-use the estimated costs of implementing the underlying scan/join
> relation, cached in the relation's PgFdwRelationInfo (ie,
> rel_startup_cost and rel_total_cost).  Since these costs wouldn't yet
> contain the costs of evaluating the final scan/join target, as tlist
> replacement by apply_scanjoin_target_to_paths() is performed afterwards.
>  So I think we need to adjust these costs so that the tlist eval costs
> are included, but ISTM that estimate_path_cost_size() forgot to do so.
> Attached is a patch for fixing this issue.

I think the following comment in apply_scanjoin_target_to_paths() should
mention that FDWs rely on the new value of reltarget.

        /*
         * Update the reltarget.  This may not be strictly necessary in all 
cases,
         * but it is at least necessary when create_append_path() gets called
         * below directly or indirectly, since that function uses the reltarget 
as
         * the pathtarget for the resulting path.  It seems like a good idea to 
do
         * it unconditionally.
         */
        rel->reltarget = llast_node(PathTarget, scanjoin_targets);

-- 
Antonin Houska
https://www.cybertec-postgresql.com

Reply via email to