Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs)

On Wed, Feb 3, 2016 at 5:56 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Feb 3, 2016 at 12:08 PM, Ashutosh Bapat
> <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>> PFA patches with naming conventions similar to previous ones.
>> pg_fdw_core_v7.patch: core changes
>> pg_fdw_join_v7.patch: postgres_fdw changes for join pushdown
>> pg_join_pd_v7.patch: combined patch for ease of testing.
>
> Hmm, I think that GetPathForEPQRecheck() is a pretty terrible name.
> How about GetExistingJoinPath()?

Oops. Hit Send too soon. Also, how about writing if
(path->param_info != NULL) continue; instead of burying the core of
the function in another level of indentation? I think you should skip
paths that aren't parallel_safe, too, and the documentation should be
clear that this will find an unparameterized, parallel-safe joinpath
if one exists.

+ ForeignPath *foreign_path;
+ foreign_path = (ForeignPath
*)joinpath->outerjoinpath;

Maybe insert a blank line between here, and in the other, similar case.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company