Re: Oddity in EXPLAIN for foreign/custom join pushdown plans

On 2016/07/28 10:01, Kouhei Kaigai wrote:
> What I'm saying is here:

> EXPLAIN (COSTS false, VERBOSE)
> SELECT t1.c1, t2.c1 FROM ft1 t1 JOIN ft2 t2 ON (t1.c1 = t2.c1) ORDER BY
> t1.c3, t1.c1 OFFSET 100 LIMIT 10;
> QUERY PLAN
> -------------------------------------------
> Limit
> Output: t1.c1, t2.c1, t1.c3
> -> Foreign Scan
> ^^^^
> Output: t1.c1, t2.c1, t1.c3
> Relations: (public.ft1 t1) INNER JOIN (public.ft2 t2)
> Remote SQL: SELECT r1."C 1", r1.c3, r2."C 1" FROM ("S 1"."T
> 1" r1 INNER JOIN "S 1"."T 1" r2 ON (((r1."C 1" = r2."C 1")))) ORDER BY
> r1.c3 ASC N\
> ULLS LAST, r1."C 1" ASC NULLS LAST
> (6 rows)

> Your concern is that EXPLAIN shows ForeignScan node as "Foreign Scan"
> although it actually works as "Foreign Join".

That may be so, but my point is that the target relations involved in
the foreign join (ie, ft1 and ft2) should be printed somewhere in the
EXPLAIN output by core, as in EXPLAIN for a simple foreign table scan.

> My suggestion makes EXPLAIN print "Foreign %s" according to the label
> assigned by the extension. Only FDW driver knows how this ForeignScan
> node actually performs on, thus, only FDW driver can set meaningful label.
>
> This label may be "Join", may be "Partial Aggregate + Join", or something
> others according to the actual job of this node.

OK, thanks for that. But sorry, I'm not sure that's a good idea. One
reason for that is that FDWs/extentions might give different labels to
the same upper-planner action, which would lead to confusing EXPLAINs.
Another is that labeling might be annoying, so some FDWs/extensions may
not want to do that.

Couldn't we print EXPLAINs in a more unified way? A simple idea is to
introduce a broad label, eg, "Foreign Processing", as a unified label;
if the FDW/extension performs any upper-planner actions remotely, then
the label "Foreign Processing" will be printed by core, and following
that, the FDW/extension would print what they want, using
ExplainForeignScan/ExplainCustomScan. Probably something like this:

Foreign Processing
Remote Operations: ...

In the Remote Operations line, the FDW/extension could print any info
about remote operations, eg, "Scan/Join + Aggregate". Different data
sources have different concepts/terms, so it seems reasonable to me that
there would be different descriptions by different data sources, to the
same plan actions done remotely.

>>> - A flag to turn on/off printing relation(s) name
>>
>> ISTM that the relation information should be always printed even in the
>> case of scanrelid=0 by the core, because that would be essential for
>> analyzing EXPLAIN results.

> We have no information which relations are actually scanned by ForeignScan
> and CustomScan. Your patch abuses fs_relids and custom_relids, however,
> these fields don't indicate the relations actually scan by these nodes.
> It just tracks relations processed by this node or its children.

Maybe my explanation was not enough, but I just mean that *joining
relations* should be printed somewhere in the EXPLAIN output for a
foreign/custom join as well, by core.

> postgres=# explain select id from t0 natural join t1 natural join t2;
> QUERY PLAN
> ---------------------------------------------------------------------------
> Custom Scan (GpuJoin) (cost=12385.67..291245.35 rows=9815465 width=4)
> GPU Projection: t0.id
> Depth 1: GpuHashJoin, HashKeys: (t0.bid)
> JoinQuals: (t0.bid = t2.bid)
> Nrows (in/out: 98.15%), KDS-Hash (size: 13.47MB, nbatches: 1)
> Depth 2: GpuHashJoin, HashKeys: (t0.aid)
> JoinQuals: (t0.aid = t1.aid)
> Nrows (in/out: 100.00%), KDS-Hash (size: 13.47MB, nbatches: 1)
> -> Seq Scan on t0 (cost=0.00..183333.96 rows=9999996 width=12)
> -> Seq Scan on t2 (cost=0.00..1935.00 rows=100000 width=4)
> -> Seq Scan on t1 (cost=0.00..1935.00 rows=100000 width=4)
> (11 rows)

> If EXPLAIN command attaches something like "on t0,t1,t2" after the GpuJoin
> *with no choice*, it is problematic and misleading.
> It shall be controllable by the extension which knows what tables are
> actually scanned. (Please note that I never says extension makes the string.
> It is a job of core explain. I suggest it needs to be controllable.)

Again, I just mean that the info "on t0,t1,t2" should be printed after
the GpuJoin label, as its joining relations.

Best regards,
Etsuro Fujita