On Thu, Oct 27, 2022 at 1:33 PM Michael Paquier <mich...@paquier.xyz> wrote: > > 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.
No. My v5 is no longer dependent on the other patch. > > > 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). Yeah, my v4 was posted along with the other GUC flag patch as a prerequisite to make the cfbot happy. This is no longer the case - v5 is a single independent patch. Sorry for the "ready for the committer" status being confusing. At that time I thought it was. > > 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. OK, I can make that adjustment if it is preferred. I think it is the same as what I already suggested a while ago [1] ("But probably it is no problem to just add #defines...") ------ [1] https://www.postgresql.org/message-id/1113448.1665717297%40sss.pgh.pa.us Kind Regards, Peter Smith. Fujitsu Australia.
