Re: Custom Scan APIs (Re: Custom Plan node)

Hi Kaigai-san,

2013/12/11 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
> 2013/12/10 Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>:
>> 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,

Check.

>> 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.

New name seems better, it is what the node express.

>> 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.

Agreed. It would be enough that Custom Scan Providers can add
arbitrary information, such as "Remote SQL" of postgres_fdw, to
EXPLAIN result via core API. Some kind of framework which helps
authors of Custom Scan Providers, but it should be considered after
the first cut.

> The attached patch fixes up a minor warning around get_rel_relkind
> and name of the property for custom-provider. Please check it.

The patch can be applied onto 2013-12-16 HEAD cleanly, and gives no
unexpected error/warinig.

I'm sorry to post separately, but I have some comments on document.

(1) ctidscan
Is session_preload_libraries available to enable the feature, like
shared_*** and local_***? According to my trial it works fine like
two similar GUCs.

(2) postgres_fdw
JOIN push--down is a killer application of Custom Scan Provider
feature, so I think it's good to mention it in the "Remote Query
Optimization" section.

Codes for core and contrib seem fine, so I'll mark the patches "Ready
for committer" after the document enhancement.

Regards,
--
Shigeru HANADA