Re: Custom/Foreign-Join-APIs (Re: [v9.5] Custom Plan API)

Kaigai-san,

2015/04/15 22:33、Kouhei Kaigai <kaigai@ak.jp.nec.com> のメール：
>> Oops, that’s right.  Attached is the revised version.  I chose fully qualified
>> name, schema.relname [alias] for the output.  It would waste some cycles during
>> planning if that is not for EXPLAIN, but it seems difficult to get a list of name
>> of relations in ExplainForeignScan() phase, because planning information has gone
>> away at that time.
>>
> I understand. Private data structure of the postgres_fdw is not designed
> to keep tree structure data (like relations join tree), so it seems to me
> a straightforward way to implement the feature.
>
> I have a small suggestion. This patch makes deparseSelectSql initialize
> the StringInfoData if supplied, however, it usually shall be a task of
> function caller, not callee.
> In this case, I like to initStringInfo(&relations) next to the line of
> initStingInfo(&sql) on the postgresGetForeignPlan. In my sense, it is
> a bit strange to pass uninitialized StringInfoData, to get a text form.
>
> @@ -803,7 +806,7 @@ postgresGetForeignPlan(PlannerInfo *root,
>     */
>    initStringInfo(&sql);
>    deparseSelectSql(&sql, root, baserel, fpinfo->attrs_used, remote_conds,
> -                    ¶ms_list, &fdw_ps_tlist, &retrieved_attrs);
> +                    ¶ms_list, &fdw_ps_tlist, &retrieved_attrs, &relations);
>
>    /*
>     * Build the fdw_private list that will be available in the executor.
>

Agreed.  If caller passes a buffer, it should be initialized by caller.  In addition to your idea, I added a check that
theRelOptInfo is a JOINREL, coz BASEREL doesn’t need relations for its EXPLAIN output. 

> Also, could you merge the EXPLAIN output feature on the main patch?
> I think here is no reason why to split this feature.

I merged explain patch into foreign_join patch.

Now v12 is the latest patch.

--
Shigeru HANADA
shigeru.hanada@gmail.com