On Wed, Oct 26, 2022 at 09:14:37PM -0500, Justin Pryzby wrote: > It seems like you're reviewing the previous version of the patch, rather > than the one attached to the message you responded to (which doesn't > have anything to do with GUC_DEFAULT_COMPILE).
It does not seem so as things stand, I have been looking at v5-0001-GUC-C-variable-sanity-check.patch as posted by Peter here: https://www.postgresql.org/message-id/cahut+puchjyxitgdtovhvdnjpbivllr49gwvs+8vwnfom4h...@mail.gmail.com In combination with a two-patch set as posted by you here: 0001-add-DYNAMIC_DEFAULT-for-settings-which-vary-by-.-con.patch 0002-WIP-test-guc-default-values.patch https://www.postgresql.org/message-id/20221024220544.gj16...@telsasoft.com These are the latest patch versions posted on their respective thread I am aware of, and based on the latest updates of each thread it still looked like there was a dependency between both. So, is that the case or not? If not, sorry if I misunderstood things. > I don't know what you meant by "make the CF bot happy" (?) It is in my opinion confusing to see that the v5 posted on this thread, which was marked as ready for committer as of https://commitfest.postgresql.org/40/3934/, seem to rely on a facility that it makes no use of. Hence it looks to me that this patch has been posted as-is to allow the CF bot to pass (I would have posted that as an isolated two-patch set with the first patch introducing the flag if need be). Anyway, per my previous comments in my last message of this thread as of https://www.postgresql.org/message-id/y1nnwftrnl3it...@paquier.xyz, I don't see a need for DYNAMIC_DEFAULT from the other thread, nor do I see a need to a style like that: +/* GUC variable */ +bool update_process_title = +#ifdef WIN32 + false; +#else + true; +#endif I think that it would be cleaner to use the same approach as checking_after_flush and similar GUCs with a centralized definition, rather than spreading such style in two places for each GUC that this patch touches (aka its declaration and its default value in guc_tables.c). In any case, the patch of this thread still needs some adjustments IMO. -- Michael
signature.asc
Description: PGP signature
