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

Kaigai-san,

2015/04/15 22:33、Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)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,
> - &params_list, &fdw_ps_tlist, &retrieved_attrs);
> + &params_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 the RelOptInfo 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(dot)hanada(at)gmail(dot)com