On 2016/08/02 22:02, Kouhei Kaigai wrote:

I wrote:
I removed the Relations line.  Here is an updated version of the patch.

* As I said upthread, I left the upper-relation handling for another
patch.  Currently, the patch prints "Foreign Scan" with no relations in
that case.

* I kept custom joins as-is.  We would need discussions about how to
choose relations we print in EXPLAIN, so I'd also like to leave that for
yet another patch.

Please don't rely on fs_relids bitmap to pick up relations to be printed.
It just hold a set of underlying relations, but it does not mean all of
these relations are actually scanned inside of the ForeignScan.

Firstly, I'd like to discuss about what the relations printed after "Foreign join" would mean. I think they would mean the relations involved in that foreign join (in other words, the ones that participate in that foreign join) rather than the relations to be scanned by a Foreign Scan representing that foreign join. Wouldn't that make sense?

In that sense I think it's reasonable to print all relations specified in fs_relids after "Foreign Join". (And in that sense I was thinking we could handle the custom join case the same way as the foreign join case.)

You didn't answer the following scenario I pointed out in the upthread.

| Please assume an enhancement of postgres_fdw that reads a small local table 
| and parse them as VALUES() clause within a remote query to execute remote join
| with foreign tables (ftbl_2, ftbl_3).
| This ForeignScan node has three underlying relations; tbl_1, ftbl_2 and 
| Likely, tbl_1 will be scanned by SeqScan, not ForeignScan itself.
| In this case, which relations shall be printed around ForeignScan?
| Is it possible to choose proper relation names without hint by the extension?
|                                                ^^^^^^^^^^^^

To care about these FDW usage, you should add an alternative bitmap rather
than fs_relids/custom_relids. My suggestion is, having explain_relids for
the purpose.

We currently don't allow such a join to be performed as a foreign join, because we allow a join to be performed so if the relations of the join are all foreign tables belonging to the same remote server, as you know.

So, as I said upthread, I think it would make sense to introduce such a bitmap when we extend the existing foreign-join pushdown infrastructure so that we can do such a thing as proposed by you. I guess that that would need to extend the concept of foreign joins, though.

Also, the logic to print "Foreign (Scan|Insert|Update|Delete)" is different
from what I suggested. I'm suggesting to allow extension giving a label
to fill up "Foreign %s" format.

Please explain why your choice is better than my proposition.

No, I haven't done anything about that yet.  I kept the behavior as-is.

At least, my proposition is available to apply on both of foreign-scan and
custom-scan, and no need to future maintenance if and when FDW gets support
remote Aggregation, Sort or others.

I'd like to discuss this issue separately, maybe in a new thread.

Best regards,
Etsuro Fujita

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to