Re: vacuum_truncate configuration parameter and isset_offset - Mailing list pgsql-hackers
| From | David G. Johnston |
|---|---|
| Subject | Re: vacuum_truncate configuration parameter and isset_offset |
| Date | |
| Msg-id | CAKFQuwasmsmA_AU3nWMkgNga=8R7MQ5vZYAi0AtZr4su=CtOqw@mail.gmail.com Whole thread Raw |
| In response to | Re: vacuum_truncate configuration parameter and isset_offset (Nathan Bossart <nathandbossart@gmail.com>) |
| Responses |
Re: vacuum_truncate configuration parameter and isset_offset
|
| List | pgsql-hackers |
On Mon, Mar 24, 2025 at 11:45 AM Nathan Bossart <nathandbossart@gmail.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.
pgsql-hackers by date: