On Thu, Feb 4, 2016 at 4:30 AM, Robert Haas <robertmh...@gmail.com> wrote:
> On Wed, Feb 3, 2016 at 5:56 PM, Robert Haas <robertmh...@gmail.com> wrote: > > On Wed, Feb 3, 2016 at 12:08 PM, Ashutosh Bapat > > <ashutosh.ba...@enterprisedb.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()? > GetExistingLocalJoinPath()? Used that. > > 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? Hmm. Done. > 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. > A query with FOR UPDATE/SHARE will be considered parallel unsafe in has_parallel_hazard_walker() and root->glob->parallelModeOK will be marked false. This implies that none of the base relations and hence join relations will be marked as consider_parallel. IIUC your logic, none of the queries with FOR UPDATE/SHARE will get a local path which is marked parallel_safe and thus join will not be pushed down. Why do you think we need to skip paths that aren't parallel_safe? I have left aside this change in the latest patches. > > + ForeignPath *foreign_path; > + foreign_path = (ForeignPath > *)joinpath->outerjoinpath; > > Maybe insert a blank line between here, and in the other, similar case. > Done. Patches attached with the previous mail. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
