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+8VwnfoM4hJTw@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.GJ16921@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/Y1nnwFTrnL3ItleP@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