On Thu, Jan 28, 2016 at 11:26 AM, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote: > 2. pg_fdw_join_v3.patch: changes to postgres_fdw - more description below
This patch no longer quite applies because of conflicts with one of your other patches that I applied today (cf. commit fbe5a3fb73102c2cfec11aaaa4a67943f4474383).
And then I broke it some more by committing a patch to extract deparseLockingClause from postgresGetForeignPlan and move it to deparse.c, but that should pretty directly reduce the size of this patch. I wonder if there are any other bits of refactoring of that sort that we can do in advance of landing the main patch, just to simplify review. But I'm not sure there are: this patch removes very little existing code; it just adds a ton of new stuff.
PFA patch to move code to deparse SELECT statement into a function deparseSelectStmtForRel(). This code is duplicated in estimate_path_cost_size() and postgresGetForeignPlan(), so moving it into a function avoids that duplication. As a side note, estimate_path_cost_size() doesn't add FOR SHARE/UPDATE clause to the statement being EXPLAINed, even if the actual statement during execution would have it. This difference looks unintentional to me. This patch corrects it as well. appendOrderByClause and appendWhereClause both create a context within themselves and pass it to deparseExpr. This patch creates the context once in deparseSelectStmtForRel() and then passes it to the other deparse functions avoiding repeated context creation.
copy_path_for_epq_recheck() and friends are really ugly. Should we consider just adding copyObject() support for those node types instead?
the note in copyfuncs.c says * We also do not support copying Path trees, mainly * because the circular linkages between RelOptInfo and Path nodes can't * be handled easily in a simple depth-first traversal.
We may avoid that by just copying the pointer to RelOptInfo and not copy the whole RelOptInfo. The other problem is paths in epq_paths will be copied as many times as the number of 2-way joins pushed down. Let me give it a try and produce patch with that.
I'm sure there's more -- this is a huge patch and I don't fully understand it yet -- but I'm out of energy for tonight.
Thanks a lot for your comments and moving this patch forward.
--
Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
