From: Antonin Houska <ah@cybertec.at>
To: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: postgres_fdw: another oddity in costing aggregate pushdown paths
Date: Fri, 22 Feb 2019 15:10:22 +0100
Message-ID: <29503.1550844622@localhost> (raw)
In-Reply-To: <5C66A056.60007@lab.ntt.co.jp>
References: <5C66A056.60007@lab.ntt.co.jp>
Etsuro Fujita <fujita.etsuro@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 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: ah@cybertec.at, fujita.etsuro@lab.ntt.co.jp
Subject: Re: postgres_fdw: another oddity in costing aggregate pushdown paths
In-Reply-To: <29503.1550844622@localhost>
* 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