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.gj16...@telsasoft.com -- Michael
signature.asc
Description: PGP signature
