On Wed, Mar 26, 2025 at 10:40 AM Nikolay Shaplov <dh...@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
