On 2016/08/05 21:47, Robert Haas wrote:
On Tue, Jul 26, 2016 at 11:20 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp> wrote:
I noticed that currently the core doesn't show any information on the target
relations involved in a foreign/custom join in EXPLAIN, by itself.

I think that's a feature, not a bug.

I agree with you.  I'd leave that for 10.0.

postgres_fdw shows the target relations in the Relations line, as shown
above, but I think that the core should show such information independently
of FDWs; in the above example replace "Foreign Scan" with "Foreign Join on
public.ft1 t1, public.ft2 t2".

I disagree with that.  Currently, when we say that something is a join
(Merge Join, Hash Join, Nested Loop) we mean that the executor is
performing a join, but that's not the case here.  The executor is
performing a scan.  The remote side, we suppose, is performing a join
for us, but we are not performing a join: we are performing a scan.
So I think the fact that it shows up in the plan as "Foreign Scan" is
exactly right.  We are scanning some foreign thing, and that thing may
internally be doing other stuff, like joins and aggregates, but all
we're doing is scanning it.

Hmm. One thing I'm concerned about would be the case where direct modification is implemented by using GetForeignUpperPaths, not PlanDirectModify. In that case, the way things are now, we would have "Foreign Scan" followed by an INSERT/UPDATE/DELETE query, but that seems odd to me.

Also, I don't really see the point of moving this from postgres_fdw to
core.  If, at some point in time, there are many FDWs that implement
sophisticated pushdown operations and we figure out that they are all
duplicating the code to do the EXPLAIN printout, and they're all
printing basically the same thing, perhaps not in an entirely
consistent way, then we could try to unify all of them into one
implementation in core.  But that's certainly not where we are right
now.  I don't see any harm at all in leaving this under the control of
the FDW, and in fact, I think it's better.  Neither the postgres_fdw
format nor what you want to replace it with are so unambiguously
awesome that some other FDW author might not come up with something
better.

I think we should leave this the way it is.

One thing we need to do to leave that as is would be to fix a bug that I pointed out upthred. Let me explain about that again. The EXPLAIN command selects relation aliases to be used in printing a query so that each selected alias is unique, but postgres_fdw wouldn't consider the uniqueness. Here is an example:

postgres=# explain verbose select * from (select t1.a, t2.a from ft1 t1, ft2 t2 where t1.a = t2.a union select t1.a, t2.a from ft1 t1, ft2 t2 where t1.a = t2.a) as t(t1a, t2a);
                                                     QUERY PLAN
--------------------------------------------------------------------------------------------------------------------
 Unique  (cost=204.12..204.13 rows=2 width=8)
   Output: t1.a, t2.a
   ->  Sort  (cost=204.12..204.12 rows=2 width=8)
         Output: t1.a, t2.a
         Sort Key: t1.a, t2.a
         ->  Append  (cost=100.00..204.11 rows=2 width=8)
               ->  Foreign Scan  (cost=100.00..102.04 rows=1 width=8)
                     Output: t1.a, t2.a
                     Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
Remote SQL: SELECT r1.a, r2.a FROM (public.t1 r1 INNER JOIN public.t2 r2 ON (((r1.a = r2.a))))
               ->  Foreign Scan  (cost=100.00..102.04 rows=1 width=8)
                     Output: t1_1.a, t2_1.a
                     Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
Remote SQL: SELECT r1.a, r2.a FROM (public.t1 r1 INNER JOIN public.t2 r2 ON (((r1.a = r2.a))))
(14 rows)

The relation aliases in the Relations line in the second Foreign Scan, t1 and t2 for ft1 and ft2, are not unique; they should be t1_1 and t2_1 (compare the aliases in the Relations line with ones in the Output line directly above that, created by core). The reason for that is because postgres_fdw has created the Relations info by using rte->eref->aliasname as relation aliases as is at path-creation time. Probably it would be a little bit too early for postgers_fdw to do that. Couldn't postgres_fdw create that info after query planning, for example, during ExplainForeignScan?

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

Reply via email to