2015-05-09 3:51 GMT+09:00 Tom Lane <t...@sss.pgh.pa.us>: > Robert Haas <robertmh...@gmail.com> writes: >> On Fri, May 8, 2015 at 1:46 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> That's nice, but 9.5 feature freeze is only a week away. I don't have a >>> lot of confidence that this stuff is actually in a state where we won't >>> regret shipping it in 9.5. > >> Yeah. The POC you were asking for upthread certainly exists and has >> for a while, or I would not have committed this. But I do not think >> it likely that the postgres_fdw support will be ready for 9.5. > > Well, we have two alternatives. I can keep hacking on this and get it > to a state where it seems credible to me, but we won't have any proof > that it actually works (though perhaps we could treat any problems > as bugs that should hopefully get found before 9.5 ships, if a > postgres_fdw patch shows up in the next few months). Or we could > revert the whole thing and bounce it to the 9.6 cycle. I don't really > like doing the latter, but I'm pretty uncomfortable with committing to > published FDW APIs that are (a) as messy as this and (b) practically > untested. The odds that something slipped through the cracks are high. > > Aside from the other gripes I raised, I'm exceedingly unhappy with the > ad-hoc APIs proposed for GetForeignJoinPaths and set_join_pathlist_hook. > It's okay for internal calls in joinpath.c to look like that, but > exporting that set of parameters seems like pure folly. We've changed > those parameter lists repeatedly (for instance in 9.2 and again in 9.3); > the odds that they'll need to change again in future approach 100%. > > One way we could reduce the risk of code breakage there is to stuff all > or most of those parameters into a struct. This might result in a small > slowdown for the internal calls, or then again maybe not --- there > probably aren't many architectures that can pass 10 parameters in > registers anyway. > Is it like a following structure definition?
typedef struct { PlannerInfo *root; RelOptInfo *joinrel; RelOptInfo *outerrel; RelOptInfo *innerrel; List *restrictlist; JoinType jointype; SpecialJoinInfo *sjinfo; SemiAntiJoinFactors *semifactors; Relids param_source_rels; Relids extra_lateral_rels; } SetJoinPathListArgs; I agree the idea. It also helps CSP driver implementation where it calls next driver that was already chained on its installation. if (set_join_pathlist_next) set_join_pathlist_next(args); is more stable manner than if (set_join_pathlist_next) set_join_pathlist_next(root, joinrel, outerrel, innerrel, restrictlist, jointype, sjinfo, semifactors, param_source_rels, extra_lateral_rels); Thanks, -- KaiGai Kohei <kai...@kaigai.gr.jp> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers