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

Hanada-san,

> I merged explain patch into foreign_join patch.
>
> Now v12 is the latest patch.
>
It contains many garbage lines... Please ensure the
patch is correctly based on the latest master +
custom_join patch.

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

> -----Original Message-----
> From: Shigeru HANADA [mailto:shigeru(dot)hanada(at)gmail(dot)com]
> Sent: Thursday, April 16, 2015 5:06 PM
> To: Kaigai Kouhei(海外 浩平)
> Cc: Ashutosh Bapat; Robert Haas; Tom Lane; Thom Brown;
> pgsql-hackers(at)postgreSQL(dot)org
> Subject: ##freemail## Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [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
>
>
>
>