Re: ISN extension - wrong volatility level for isn_weak() function

Guess I shouldn’t have said it was easy considering all the edits needed hehe.

Thanks a lot for the help, fixes and push Tom! Wasn’t expecting such a lightning turnaround.

/Viktor Holmberg
On 16 Mar 2025 at 18:02 +0000, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, wrote:
> Viktor Holmberg <v(at)viktorh(dot)net> writes:
> > Oh, sorry! Don’t know what I was doing there - not used to this patch based workflow. Here comes the real patch.
> > This now uses the GUC - that was a lot easier than I thought.
>
> I reviewed this and pushed it with some corrections. Thanks for
> the contribution!
>
> Some notes:
>
> * To make g_weak actually act like a GUC, accept_weak_input() has
> to set it via set_config_option() not just overwrite it. Otherwise
> it won't roll back on transaction abort, for example. There was also
> a missing MarkGUCPrefixReserved() call.
Makes sense. I actually thought about calling set_config option but decided not fixing it was more backwards compatible.
But can’t think of any reason why anyone wouldn’t want it working like a true GUC.
>
> * I concluded that the isn_weak() functions ought to be marked like
> set_config() (VOLATILE and PARALLEL UNSAFE) and current_setting()
> (STABLE and PARALLEL SAFE). It's debatable whether a function that
> reacts to a GUC should really be STABLE, since you can surely make its
> output change intra-query if you try. However, we pretty consistently
> do that elsewhere because of the negative performance implications of
> marking all such functions VOLATILE. The previous PARALLEL RESTRICTED
> markings were probably okay when g_weak's value couldn't propagate into
> parallel workers, but they're wrong now.
Makes sense
>
> * You missed updating the module's meson.build file, which is hardly
> your fault since I pointed you at an example commit that predated
> our Meson support. But nowadays pretty much anything done to a
> Makefile has to be reflected in meson.build and vice versa.
>
> * I editorialized on the docs a bit, mostly to be more like existing
> practice in other contrib modules. One point there is that "GUC"
> is internal jargon that we prefer to avoid using in user-facing docs.
Agree you edits make it more clear.
>
> All in all though, pretty close for a first contribution.
> Thanks again!
>
> regards, tom lane