On Wed, Mar 26, 2025 at 10:40 AM Nikolay Shaplov <dhyan@nataraj.su> wrote:
> Then please, listen to a reason. If we add isset_offset, it will make code
> inconsistent, because it follow same purpose as unreachable default_value in
> other non boolean options. Having two conflicting ways to do same thing is a
> bad design.
As Nathan also says, I'm not sure why we're obliged to do everything
one way or the other.
If you look at postgresql.conf, there are some settings where a
designated value is used to disable something -- for instance
temp_file_limit=-1 disables any limit on the size of temp files, and
backend_flush_after=0 also disables the corresponding behavior -- but
there are other settings where that doesn't make sense and we add a
separate switch e.g. ssl=off disables SSL, separate from any of the
other SSL-affecting GUCs, and archive_mode=off disables archiving
separately from the archive_command setting. Your argument seems to
lead to the conclusion that it is bad design, but I don't see it that
way. I think it would be bad design to add a separate
temp_file_limit_enabled=on/off setting when we could just use
temp_file_limit=-1 to mean that, and I think it would be equally a bad
idea to enable ssl based on some non-obvious criterion like whether
ssl_cert_file is set to the empty string. The cases are different, and
it's OK to treat them differently, IMHO.
> We can get rid of unreachable default_value in every option and use
> isset_offset everywhere to detect unset options, but this will give us lot's of
> extra flags in StdRdOptions,
I agree, that doesn't seem like a good idea.
> and more over we still have case when we have
> useful default_value. So code will have to either ignore default_value in
> option definition when author imply that he will use isset_offset flag later, or
> ignore isset_offset flag when processing option with useful default value. These
> two cases, I emphasize, are not directly stated in the code, they are implied
> somewhere in author's head, and you will never guess which one is in this
> case, until you finish reading, or one need to write a lot of comments.
I'm surprised that you think it will be hard to clarify this in the comments.
--
Robert Haas
EDB: http://www.enterprisedb.com
