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.
>
Why? According to your rule, Hash Join should take "on t0,t1,t2".

postgres=# explain select id from t0 natural join t1 natural join t2;
QUERY PLAN
-----------------------------------------------------------------------------
Hash Join (cost=6370.00..4560826.24 rows=98784048 width=4)
Hash Cond: (t0.aid = t1.aid)
-> Hash Join (cost=3185.00..3199360.58 rows=98784048 width=8)
Hash Cond: (t0.bid = t2.bid)
-> Seq Scan on t0 (cost=0.00..1833334.80 rows=100000080 width=12)
-> Hash (cost=1935.00..1935.00 rows=100000 width=4)
-> Seq Scan on t2 (cost=0.00..1935.00 rows=100000 width=4)
-> Hash (cost=1935.00..1935.00 rows=100000 width=4)
-> Seq Scan on t1 (cost=0.00..1935.00 rows=100000 width=4)
(9 rows)

> > 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.
>
If extension put a label hard to understand, it is a problem of the extension.

> Another is that labeling might be annoying, so some FDWs/extensions may
> not want to do that.
>
I'm happy with arbitrary label, not annoying. :-)

> 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.
>
"Foreign" implies this node is processed by FDW, but "Procesing" gives us
no extra information; seems to me redundant.
Prior to the new invention, please explain why you don't want to by my
suggestion first? Annoying is a feel of you, but not a logic to persuade
others.

> >>> - 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.
>
I don't want. Your rule does not match existing EXPLAIN output convention.
Please see an EXPLAIN output that involves only base relations.

My largest concern for you proposition is, ForeignScan/CustomScan node is
enforced to print name of underlying relations, regardless of its actual
behavior. The above GpuJoin never scans tables at least, thus, it mislead
users if we have no choice to print underlying relation names.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>