On 2016/02/04 8:00, Robert Haas 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.

Thank you for working on this, Ashutosh and Robert! I've not look at the patches closely yet, but ISTM the patches would be really in good shape!

Hmm, I think that GetPathForEPQRecheck() is a pretty terrible name.
How about GetExistingJoinPath()?

+1

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.

* Is it safe to replace outerjoinpath with its fdw_outerpath the following way? I think that if the join relation represented by outerjoinpath has local conditions that can't be executed remotely, we have to keep outerjoinpath in the path tree; we will otherwise fail to execute the local conditions. No?

+                       /*
+                        * If either inner or outer path is a ForeignPath 
corresponding to
+                        * a pushed down join, replace it with the 
fdw_outerpath, so that we
+                        * maintain path for EPQ checks built entirely of local 
join
+                        * strategies.
+                        */
+                       if (IsA(joinpath->outerjoinpath, ForeignPath))
+                       {
+                               ForeignPath *foreign_path;
+                               foreign_path = (ForeignPath 
*)joinpath->outerjoinpath;
+                               if (foreign_path->path.parent->reloptkind == 
RELOPT_JOINREL)
+                                       joinpath->outerjoinpath = 
foreign_path->fdw_outerpath;
+                       }

* IIUC, that function will be used by custom joins, so I think it would be better to put that function somewhere in the /optimizer directory (pathnode.c?).

Best regards,
Etsuro Fujita




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to