On Wed, Oct 26, 2022 at 06:31:56PM +1100, Peter Smith wrote:
> I re-checked all the GUC C vars which your patch flags as
> GUC_DEFAULT_COMPILE. For some of them, where it was not any trouble, I
> made the C var assignment use the same preprocessor rules as used by
> guc_tables. For others (mostly the string ones) I left the GUC C var
> untouched because the sanity checker function already has a rule not
> to complain about int GUC C vars which are 0 or string GUC vars which
> are NULL.
I see. So you have on this thread an independent patch to make the CF
bot happy, still depend on the patch posted on [1] to bypass the
changes with variables whose boot values are compilation-dependent.
Is it right to believe that the only requirement here is
GUC_DEFAULT_COMPILE but not GUC_DEFAULT_INITDB? The former is much
more intuitive than the latter. Still, I see an inconsistency here in
what you are doing here.
sanity_check_GUC_C_var() would need to skip all the GUCs marked as
GUC_DEFAULT_COMPILE, meaning that one could still be "fooled by a
mismatched value" in these cases. We are talking about a limited set
of them, but it seems to me that we have no need for this flag at all
once the default GUC values are set with a #defined'd value, no?
checkpoint_flush_after, bgwriter_flush_after, port and
effective_io_concurrency do that, which is why
v5-0001-GUC-C-variable-sanity-check.patch does its stuff only for
maintenance_io_concurrency, update_process_title, assert_enabled and
syslog_facility. I think that it would be simpler to have a default
for these last four with a centralized definition, meaning that we
would not need a GUC_DEFAULT_COMPILE at all, while the validation
could be done for basically all the GUCs with default values
assigned. In short, this patch has no need to depend on what's posted
in [1].
[1]: https://www.postgresql.org/message-id/20221024220544.GJ16921@telsasoft.com
--
Michael
