Re: vacuum_truncate configuration parameter and isset_offset

On Mon, Mar 24, 2025 at 11:45 AM Nathan Bossart <nathandbossart(at)gmail(dot)com>
wrote:

> On Mon, Mar 24, 2025 at 09:03:11PM +0300, Nikolay Shaplov wrote:
> > В письме от понедельник, 24 марта 2025 г. 20:48:29 MSK пользователь
> Nathan
> > Bossart написал:
> >
> >> For the sake of discussion, here is what the enum approach would look
> like.
> >
> > In my point of view this solution is much-much better: it achieves all
> goals,
> > has no drawbacks, and do not change reloption engine.

I am in favor of changing over to an enum at this point (let's try to keep
users believing its a boolean if possible).

This isn't an endorsement on this idea and framing of things (say were we
in a green-field situation) but rather that this patch is basically the
"null hypothesis" and the alternate (presently committed) patch doesn't do
enough to reject the null hypothesis, IME. (More detail on this below.)

>
> I think this change would probably work fine, but it does have a couple of
> drawbacks:
>
> * We'll need to make it really clear in comments why you would want to use
> this enum instead of making it a Boolean reloption. That's not a big
> deal.
>

Agreed.

> * We'd need to decide what to say on the documentation side. My first
> instinct is that we should just leave it as "boolean" because otherwise
> we're going to describe something that sounds an awful lot like a
> Boolean.
>

I'd start here too and wait to be convinced its pointless because the user
is going to be exposed to the enum aspect anyway via system introspection.

>
> * I don't think this matches the parse_bool() behavior exactly. For
> example, parse_bool() appears to accept inputs like "t" to mean "true".
> This is also probably not a huge deal.
>

Fixable for sure.

> That being said, I'm open to applying this patch if it seems like a
> majority of folks prefer this implementation. FWIW I'm still partial to
> the isset_offset approach for the reasons I've already discussed.
>
>
I'm preferable to turning the implementation into an enum. Slight
preference to documenting it as the boolean it presently is; pending any
information regarding where in user-space, particularly in psql but also
any client-side details, this internal detail might show through. It
avoids having to tell the reader that we choose enum in order to
accommodate optionality. But it isn't the end of the world if we do say
that either.

I'm not convinced our existing way of approaching/framing this is great -
defaults and is-set are better treated as being orthogonal; but, I'm also
not fond of reserving space for a bunch of zeros and having to using naming
conventions to match up vacuum_truncate and vacuum_truncate_set. So while
"isset_offset" better matches my preferred "framing" of this system I agree
forcing it in here with an implementation that isn't ideal doesn't make
sense when we can just continue on doing what we've been doing and use this
experience to bring a fresh perspective to the larger work that apparently
is going on in this area.

We do at least have the user experience correct either way - use reset to
change to the state that denotes "this option has not been set".

David J.