Allow use_remote_estimate to be set to 0.

[deathwish]

Previously, the code applying default settings would assume the option was unset, and set the default value of 1. We use a sentinel value instead, which allows us to detect this case separately.

[jenkins-juliogonzalez]

Can one of the admins verify this patch?

[GeoffMontee]

Test this, please

[jenkins-juliogonzalez]

Test FAILed.

[GeoffMontee]

Hi @deathwish,

Thanks for the patch! It looks like there are some test failures. Could you please fix those? For example, see here: https://jenkins.juliogonzalez.es/job/tds_fdw-build-pr/15/DISTRO=centos7,PG_VER=11,label=docker/console

In tdsValidateForeignTableOptionSet(TdsFdwOptionSet *option_set), it looks to me like this condition might need to be tweaked:

if (option_set->use_remote_estimate < 0 || option_set->use_remote_estimate > 1)

Isn't the following condition correct for this context instead?:

if (option_set->use_remote_estimate < -1 || option_set->use_remote_estimate > 1)

[deathwish]

if (option_set->use_remote_estimate < -1 || option_set->use_remote_estimate > 1)

The -1 value is supposed to be replaced by the option's default value in all cases, so I view the test failures as indicative. Ensuring defaults are set before option validation in all cases, as the commit I just pushed does, seems to fix the test failures and likely fixes other problems.

ETA: It's possible that tdsGetForeignServerTableOptions should also validate the range of use_remote_estimate. It would be reasonable to allow -1 in this case to explicitly select the default behavior.

[GeoffMontee]

Test this, please

[jenkins-juliogonzalez]

Test PASSed.

[GeoffMontee]

This has been merged. Thanks again for the patch!

[deathwish]

Great! Glad I could help, thanks for getting this in.