From: | Antonin Houska <ah(at)cybertec(dot)at> |
---|---|
To: | Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Problems with plan estimates in postgres_fdw |
Date: | 2019-02-22 13:54:29 |
Message-ID: | 29190.1550843669@localhost |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> Maybe, my explanation in that thread was not enough, but the changes proposed
> by the patch posted there wouldn't obsolete the above. Let me explain using a
> foreign-table variant of the example shown in the comments for
> make_group_input_target():
>
> SELECT a+b, sum(c+d) FROM foreign_table GROUP BY a+b;
>
> When called from postgresGetForeignPaths(), the reltarget for the base
> relation foreign_table would be {a, b, c, d}, for the same reason as the LIMIT
> example in [1], and the foreign_table's rel_startup_cost and rel_total_cost
> would be calculated based on this reltarget in that callback routine. (The
> tlist eval costs would be zeroes, though). BUT: before called from
> postgresGetForeignUpperPaths() with the UPPERREL_GROUP_AGG stage, the
> reltarget would be replaced with {a+b, c, d}, which is the final scan target
> as explained in the comments for make_group_input_target(), and the eval costs
> of the new reltarget wouldn't be zeros, so the costs of scanning the
> foreign_table would need to be adjusted to include the eval costs. That's why
> I propose the assignments in estimate_path_cost_size() shown above.
Yes, apply_scanjoin_target_to_paths() can add some more costs, including those
introduced by make_group_input_target(), which postgres_fdw needs to account
for. This is what you fix in
https://www.postgresql.org/message-id/5C66A056.60007%40lab.ntt.co.jp
> On the other hand, the code bit added by
> 0001-postgres_fdw-Perform-UPPERREL_ORDERED-step-remotely-v3.patch handles the
> case where a post-scan/join processing step other than grouping/aggregation
> (ie, the final sort or LIMIT/OFFSET) is performed directly on a base or join
> relation, as in the LIMIT example. So, the two changes are handling different
> cases, hence both changes would be required.
Do you mean this part?
+ /*
+ * If this includes an UPPERREL_ORDERED step, the given target, which
+ * would be the final target to be applied to the resulting path, might
+ * have different expressions from the underlying relation's reltarget
+ * (see make_sort_input_target()); adjust tlist eval costs.
+ */
+ if (fpextra && fpextra->target != foreignrel->reltarget)
+ {
+ QualCost oldcost = foreignrel->reltarget->cost;
+ QualCost newcost = fpextra->target->cost;
+
+ startup_cost += newcost.startup - oldcost.startup;
+ total_cost += newcost.startup - oldcost.startup;
+ total_cost += (newcost.per_tuple - oldcost.per_tuple) * rows;
+ }
You should not need this. Consider what grouping_planner() does if
(!have_grouping && !activeWindows && parse->sortClause != NIL):
sort_input_target = make_sort_input_target(...);
...
grouping_target = sort_input_target;
...
scanjoin_target = grouping_target;
...
apply_scanjoin_target_to_paths(...);
So apply_scanjoin_target_to_paths() effectively accounts for the additional
costs introduced by make_sort_input_target().
Note about the !activeWindows test: It occurs me now that no paths should be
added to UPPERREL_ORDERED relation if the query needs WindowAgg node, because
postgres_fdw currently cannot handle UPPERREL_WINDOW relation. Or rather in
general, the FDW should not skip any stage just because it doesn't know how to
build the paths.
--
Antonin Houska
https://www.cybertec-postgresql.com
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2019-02-22 14:00:47 | Re: Checksum errors in pg_stat_database |
Previous Message | Claudio Freire | 2019-02-22 13:34:42 | Re: Using old master as new replica after clean switchover |