On 7 March 2017 at 01:22, Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> wrote: > On Mon, Mar 6, 2017 at 1:29 PM, David Rowley > <david.row...@2ndquadrant.com> wrote: >> On 6 March 2017 at 18:51, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote: >>> On 2017/03/06 11:05, David Rowley wrote: >> It seems like a much better idea to keep the server option processing >> in one location, which is what I did. > > I agree with this. However > 1. apply_server_options() is parsing the options strings again and > again, which seems wastage of CPU cycles. It should probably pick up > the options from one of the joining relations. Also, the patch calls > GetForeignServer(), which is not needed; in foreign_join_ok(), it will > copy it from the outer relation anyway. > 2. Some server options like use_remote_estimate and fetch_size are > overridden by corresponding table level options. For a join relation > the values of these options are derived by some combination of > table-level options.
This seems much more sane. I'd failed to find the code which takes the largest fetch_size. > I think we should write a separate function > apply_fdw_options_for_joinrel() and pass the fpinfos of joinrel, outer > and inner relation. The function will copy the values of server level > options and derive values for table level options. We would add a note > there to keep this function in sync with apply_*_options(). I don't > think there's any better way to keep the options in sync for base > relations and join relations. > > Here's the patch attached. Agreed. It seems like a good idea to keep that logic in a single location I've beaten your patch around a bit and come up with the attached. The changes from yours are mostly cosmetic, but I've also added a regression test too. What do you think? -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Description: Binary data
-- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers