On 2017/01/27 20:04, Ashutosh Bapat wrote:
On Fri, Jan 13, 2017 at 12:30 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
A more clean way I'm thinking is: (1) in
postgresGetForeignJoinPaths(), create a tlist by build_tlist_to_deparse()
and save it in fpinfo->tlist before estimate_path_cost_size() if
use_remote_estimates=true, (2) in estimate_path_cost_size(), use the
fpinfo->tlist if use_remote_estimates=true, and (3) in
postgresGetForeignPlan(), use the fpinfo->tlist as the fdw_scan_tlist if
use_remote_estimates=true, otherwise create a tlist as the fdw_scan_tlist by
build_tlist_to_deparse(), like the attached.
What do you think about that?
Another change is: I simplified build_tlist_to_deparse() a bit and put that
in postgres_fdw.c because that is used only in postgres_fdw.c.
I still think we should discuss this separately because this is an existing
issue and that makes it easy to review the patch. If the attached is the
right way to go, I'll update the join-pushdown patch on top of the patch.
I don't think it's right to assume that the targetlist will be
available only when use_remote_estimate is true; for grouping it's
certainly not.
My explanation was not enough. Sorry about that. My proposal described
above was for join relations, not upper relations. We build the
targetlist of an upper relation during postgresGetForeignUpperPaths, so
for grouping I think we should use that targetlist in
estimate_path_cost_size and postgresGetForeignPlan. The patch was
created that way.
But I don't see this discussion getting anywhere. I will leave it to
the committer's judgement.
I'm fine with that.
I think we should pick up your patch on
27th December, update the comment per your mail on 5th Jan. I will
review it once and list down the things left to committer's judgement.
Sorry, I started thinking we went in the wrong direction. I added to
deparseSelectStmtForRel build_subquery_tlists, which creates a tlist for
each subquery present in a given join tree prior to deparsing a whole
remote query. But that's nothing but an overhead; we need to create a
tlist for the top-level query because we use it as fdw_scan_tlist, but
not for subqueries, and we need to create retrieved_attrs for the
top-level query while deparsing the targetlist in
deparseExplicitTargetList, but not for subqueries. So, we should need
some work to avoid such a useless overhead.
Best regards,
Etsuro Fujita
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
