On 6 March 2017 at 18:51, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:
> On 2017/03/06 11:05, David Rowley wrote:
>> The attached patch, based on 9.6, fixes the problem by properly
>> processing the foreign server options in
>> postgresGetForeignJoinPaths().
>
> I think the fix would work well, but another way I think is much simpler and
> more consistent with the existing code is to (1) move code for getting the
> server info from the outer's fpinfo before calling is_foreign_expr() in
> foreign_join_ok() and (2) add code for getting the shippable extensions info
> from the outer's fpinfo before calling that function, like the attached.
Thanks for looking over my patch.
I looked over yours and was surprised to see:
+ /* is_foreign_expr might need server and shippable-extensions info. */
+ fpinfo->server = fpinfo_o->server;
+ fpinfo->shippable_extensions = fpinfo_o->shippable_extensions;
That appears to do nothing for the other server options. For example
use_remote_estimate, which is used in estimate_path_cost_size(). As of
now, that's also broken. My patch fixes that problem too, yours
appears not to.
Even if you expanded the above code to process all server options, if
someone comes along later and adds a new option, my code will pick it
up, yours could very easily be forgotten about and be the cause of yet
more bugs.
It seems like a much better idea to keep the server option processing
in one location, which is what I did.
Do you think differently?
-- David Rowley http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
