On Fri, Mar 17, 2017 at 9:15 AM, Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> wrote: > This set of patches fixes both of those things.
0001 changes the purpose of a function and then 0007 renames it. It would be better to include the renaming in 0001 so that you're not taking multiple whacks at the same function in the same patch series. I believe it would also be best to include 0011's changes to adjust_appendrel_attrs_multilevel in 0001. 0002 should either add find_param_path_info() to the relevant header file as extern from the beginning, or it should declare and define it as static and then 0007 can remove those markings. It makes no sense to declare it as extern but put the prototype in the .c file. 0004 still needs to be pared down. If you want to get something committed this release cycle, you have to get these details taken care of, uh, more or less immediately. Actually, preferably, several weeks ago. You're welcome to maintain your own test suite locally but what you submit should be what you are proposing for commit -- or if not, then you should separate the part proposed for commit and the part included for dev testing into two different patches. In 0005's README, the part about planning partition-wise joins in two phases needs to be removed. This patch also contains a small change to partition_join.sql that belongs in 0004. 0008 removes direct tests against RELOPT_JOINREL almost everywhere, but it overlooks the new ones added to postgres_fdw.c by b30fb56b07a885f3476fe05920249f4832ca8da5. It should be updated to cover those as well, I suspect. The commit message claims that it will "Similarly replace RELOPT_OTHER_MEMBER_REL test with IS_OTHER_REL() where we want to test for child relations of all kinds, but in fact it makes exactly zero such substitutions. While I was studying what you did with reparameterize_path_by_child(), I started to wonder whether reparameterize_path() doesn't need to start handling join paths. I think it only handles scan paths right now because that's the only thing that can appear under an appendrel created by inheritance expansion, but you're changing that. Maybe it's not critical -- I think the worst consequences of missing some handling there is that we won't consider a parameterized path in some case where it would be advantageous to do so. Still, you might want to investigate a bit. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers