Hanada-san,
Thanks for your reviewing.
2013/12/10 Shigeru Hanada <shigeru.hanada@gmail.com>:
> Hi KaiGai-san,
>
> 2013/12/8 Kohei KaiGai <kaigai@kaigai.gr.jp>:
>> The attached patches include documentation fixup by Hanada-san,
>> and relocation of is_managed_relation (the portion to check whether
>> the relation is a foreign table managed by a particular FDW) and
>> has_wholerow_reference.
>> I didn't touch the EXPLAIN logic because I'm uncertain whether the
>> cost of remote join is reasonable towards the cost as an alternative
>> path to local joins.
>>
>> Please check it. Thanks,
>
> The patches could be applied cleanly, but I saw a compiler warning
> about get_rel_relkind() in foreign.c, but it's minor issue. Please
> just add #include of utils/lsyscache.h there.
>
Fixed,
> I have some more random comments about EXPLAIN.
>
> 1) You use "Operation" as the label of Custom Scan nodes in non-text
> format, but it seems to me rather "provider name". What is the string
> shown there?
>
I tried free-riding on the existing properties, but it does not make sense
indeed, as you pointed out.
I adjusted the explain.c to show "Custom-Provider" property for Custom-
Scan node, as follows.
postgres=# explain(format xml) select * from t1 where ctid > '(4,0)'::tid;
QUERY PLAN
----------------------------------------------------------
<explain xmlns="http://www.postgresql.org/2009/explain">+
<Query> +
<Plan> +
<Node-Type>Custom Scan</Node-Type> +
<Custom-Provider>ctidscan</Custom-Provider> +
<Relation-Name>t1</Relation-Name> +
<Alias>t1</Alias> +
<Startup-Cost>0.00</Startup-Cost> +
<Total-Cost>12.30</Total-Cost> +
<Plan-Rows>410</Plan-Rows> +
<Plan-Width>36</Plan-Width> +
<Filter>(ctid > '(4,0)'::tid)</Filter> +
</Plan> +
</Query> +
</explain>
(1 row)
> 2) It would be nice if we can see the information about what the
> Custom Scan node replaced in EXPLAIN output (even only in verbose
> mode). I know that we can't show plan tree below custom scan nodes,
> because CS Provider can't obtain other candidates. But even only
> relation names used in the join or the scan would help users to
> understand what is going on in Custom Scan.
>
Even though I agree that it helps users to understand the plan,
it also has a headache to implement because CustomScan node
(and its super class) does not have an information which relations
are underlying. Probably, this functionality needs to show
the underlying relations on ExplainTargetRel() if CustomScan node
represents a scan instead of join. What data source can produce
the list of underlying relations here?
So, if it is not a significant restriction for users, I'd like to work on this
feature later.
The attached patch fixes up a minor warning around get_rel_relkind
and name of the property for custom-provider. Please check it.
Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
