>> >> The new name merge_fdw_options() is shorter than the one I chose, but >> we are not exactly merging options for an upper relation since there >> isn't the other fpinfo to merge from. But I think we can live with >> merge_fdw_options(). > > Perhaps "combine" is a better word? I didn't really see a problem with > that. After I posted this I wished I'd made the function even more > generic to accept either side as NULL and make up the new fpinfo from > the non-NULL one, or Merge if both were non-NULL. I liked that way > much better than giving the function too much knowledge of what its > purpose actually is. It's more likely to get used for something else > in the future, which means there's less chance that someone else will > make the same mistake.
It's more like copy for an upper rel and merge for a join rel. Your point about making it side-agnostic is good but I doubt if we want to spend more effort there. If somebody writes a code with the input plan stuffed in the innerrel instead of the outerrel, s/he will notice it immediately when testing as assertion would fail or there will be a straight segfault. We will decide what to do then. > >> Once I fixed that, the testcases started showing an assertion failure, >> since fpinfo of a base relation can not have an outerrel. Fixed the >> assertion as well. If we are passing fpinfo's of joining relations, >> there's no need to have outerrel and innerrel in fpinfo of join. > > Looks like you forgot to update the comment on the Assert() Yes and I also forgot to update the function prologue to refer to the fpinfo_o/i instead of inner and outer relations. Attached patch corrects it. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
foreign_outerjoin_pushdown_fix_v4.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
