Thread: Postgres and --config-file option
Hi, A friend of mine complained about strange behavior of `postgres`. When executed without any arguments the following error is shown: ``` $ postgres postgres does not know where to find the server configuration file. You must specify the --config-file or -D invocation option or set the PGDATA environment variable. ``` However --config-file is not listed in --help output. Apparently that's because it's not a regular option but a GUС. It is in fact supported: ``` $ postgres --config-file=/tmp/fake.txt postgres: could not access the server configuration file "/tmp/fake.txt": No such file or directory ``` Additionally --help says: ``` [...] Please read the documentation for the complete list of run-time configuration settings and how to set them on the command line or in the configuration file ``` ... which personally I don't find extremely useful to be honest. OK, let's check section "20.1.4. Parameter Interaction via the Shell" [1] of the documentation. Currently it doesn't tell anything about the ability to specify GUCs --like-this, unless I missed something. Should we remove --config-file from the error message to avoid any confusion? Should we correct --help output? Should we update the documentation? [1]: https://www.postgresql.org/docs/current/config-setting.html#CONFIG-SETTING-SHELL -- Best regards, Aleksander Alekseev
On Sat, Jan 13, 2024 at 01:39:50PM +0300, Aleksander Alekseev wrote: > OK, let's check section "20.1.4. Parameter Interaction via the Shell" > [1] of the documentation. Currently it doesn't tell anything about the > ability to specify GUCs --like-this, unless I missed something. It appears to be documented for 'postgres' as follows [0]: --name=value Sets a named run-time parameter; a shorter form of -c. and similarly within the --help output: --NAME=VALUE set run-time parameter Its documentation also describes this method of specifying parameters in the 'Examples' section. The section you refer to calls out "-c", so it is sort-of indirectly mentioned, but that might be a bit of a generous assessment. > Should we remove --config-file from the error message to avoid any > confusion? Should we correct --help output? Should we update the > documentation? It might be worthwhile to update the documentation if it would've helped prevent confusion here. Separately, I noticed that this is implemented in postmaster.c by looking for the '-' option character returned by getopt(), and I'm wondering why this doesn't use getopt_long() instead. AFAICT this dates back to the introduction of GUCs in 6a68f426 (May 2000). [0] https://www.postgresql.org/docs/devel/app-postgres.html -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On Saturday, January 13, 2024, Nathan Bossart <nathandbossart@gmail.com> wrote:
On Sat, Jan 13, 2024 at 01:39:50PM +0300, Aleksander Alekseev wrote:
> Should we remove --config-file from the error message to avoid any
> confusion? Should we correct --help output? Should we update the
> documentation?
It might be worthwhile to update the documentation if it would've helped
prevent confusion here.
Pointing out the long form in the -c definition makes sense.
As for the help message, I’d minimally add:
“You must specify the --config-file (or equivalent -c) or -D invocation …”
I’m fine with the status quo regarding the overview documentation mentioning both forms. I also haven’t tested whether PGOPTIONS accepts both forms or only the -c form as presently documented. Presently the —name=value form seems discouraged in favor of -c which I’m ok with and trying to mention both everywhere seems needlessly verbose. But I’d be interested in reviewing against an informed patch improving this area more broadly than dealing with this single deviant usage. I do like this specific usage of the long-form option.
David J.
Hi, > It might be worthwhile to update the documentation if it would've helped > prevent confusion here. > Its documentation also describes this method of specifying parameters in > the 'Examples' section. I believe the documentation for 'postgres' already does a decent job in describing what --NAME=VALUE means, and gives an example. IMO the actual problem is with --help message and the specific error message. > Please read the documentation for the complete list of run-time > configuration settings and how to set them on the command line or in > the configuration file Additionally --help message doesn't tell which part of the documentation should be read specifically. This being said, personally I don't think that providing specific URLs in the --help message would be a good idea. This would indicate that the --help message is just written poorly. > As for the help message, I’d minimally add: > > “You must specify the --config-file (or equivalent -c) or -D invocation …” Good idea. PFA the patch. It's short but I think it mitigates the problem. -- Best regards, Aleksander Alekseev
Attachment
On Mon, Jan 15, 2024 at 4:35 AM Aleksander Alekseev <aleksander@timescale.com> wrote:
PFA the patch. It's short but I think it mitigates the problem.
I took a look at where these options are discussed in the documentation and now feel that we should make these options clear more broadly (config and libpq, plus pointing to --name from -c in a couple of places). It doesn't add much verbosity and, frankly, if I was to pick one "--name=value" would win and so I'd rather document it, leaving -c alone for historical reasons.
I've attached a replacement patch with the additional changes.
David J.
Attachment
On Fri, Feb 2, 2024 at 2:23 PM David G. Johnston <david.g.johnston@gmail.com> wrote:
On Mon, Jan 15, 2024 at 4:35 AM Aleksander Alekseev <aleksander@timescale.com> wrote:PFA the patch. It's short but I think it mitigates the problem.I took a look at where these options are discussed in the documentation and now feel that we should make these options clear more broadly (config and libpq, plus pointing to --name from -c in a couple of places). It doesn't add much verbosity and, frankly, if I was to pick one "--name=value" would win and so I'd rather document it, leaving -c alone for historical reasons.I've attached a replacement patch with the additional changes.
And I just saw one more apparently undocumented requirement (or a typo)
You must specify the --config-file
The actual parameter is "config_file", so apparently we are supposed to either convert underscores to hyphens or we have a typo.
David J.
On 02.02.24 22:27, David G. Johnston wrote: > On Fri, Feb 2, 2024 at 2:23 PM David G. Johnston > <david.g.johnston@gmail.com <mailto:david.g.johnston@gmail.com>> wrote: > > On Mon, Jan 15, 2024 at 4:35 AM Aleksander Alekseev > <aleksander@timescale.com <mailto:aleksander@timescale.com>> wrote: > > PFA the patch. It's short but I think it mitigates the problem. > > > I took a look at where these options are discussed in the > documentation and now feel that we should make these options clear > more broadly (config and libpq, plus pointing to --name from -c in a > couple of places). It doesn't add much verbosity and, frankly, if I > was to pick one "--name=value" would win and so I'd rather document > it, leaving -c alone for historical reasons. > > I've attached a replacement patch with the additional changes. > > > And I just saw one more apparently undocumented requirement (or a typo) > > You must specify the --config-file > > The actual parameter is "config_file", so apparently we are supposed to > either convert underscores to hyphens or we have a typo. We convert '-' to '_' when parsing long options (see ParseLongOption() in guc.c). So writing the long options with hyphens should generally be preferred in documentation.
David, Peter, > > The actual parameter is "config_file", so apparently we are supposed to > > either convert underscores to hyphens or we have a typo. > > We convert '-' to '_' when parsing long options (see ParseLongOption() > in guc.c). So writing the long options with hyphens should generally be > preferred in documentation. Thanks for all your great input. Here is the updated patch. -- Best regards, Aleksander Alekseev
Attachment
Hi, > Thanks for all your great input. Here is the updated patch. Here is the patch v4 with fixed typo ("geoq"). Per off-list feedback from Alvaro - thanks! -- Best regards, Aleksander Alekseev
Attachment
On Tue, May 14, 2024 at 03:03:58PM +0300, Aleksander Alekseev wrote: > Here is the patch v4 with fixed typo ("geoq"). Per off-list feedback > from Alvaro - thanks! + <option>-c name=value</option> command-line parameter, or its equivalent + <option>--name=value</option> variation. For example, <programlisting> -postgres -c log_connections=yes -c log_destination='syslog' +postgres -c log_connections=yes --log-destination='syslog' Wow. I've used -c many times, and never noticed that this was a supported option switch. There's always something to learn around here.. - printf(_(" -c NAME=VALUE set run-time parameter\n")); + printf(_(" -c NAME=VALUE set run-time parameter (see also --NAME)\n")); [...] - printf(_(" --NAME=VALUE set run-time parameter\n")); + printf(_(" --NAME=VALUE set run-time parameter, a shorter form of -c\n")); [...] - to set multiple parameters. + to set multiple parameters. See the <option>--name</option> + option below for an alternate syntax. [...] - Sets a named run-time parameter; a shorter form of - <option>-c</option>. + Sets the named run-time parameter; a shorter form of + <option>-c</option>. See <xref linkend="runtime-config"/> + for a listing of parameters. Not sure that these additions in --help or the docs are necessary. The rest looks OK. - "You must specify the --config-file or -D invocation " + "You must specify the --config-file (or equivalent -c) or -D invocation " How about "You must specify the --config-file, -c \"config_file=VALUE\" or -D invocation"? There is some practice for --opt=VALUE in .po files. -- Michael
Attachment
On 15.05.24 04:07, Michael Paquier wrote: > Not sure that these additions in --help or the docs are necessary. > The rest looks OK. > > - "You must specify the --config-file or -D invocation " > + "You must specify the --config-file (or equivalent -c) or -D invocation " > > How about "You must specify the --config-file, -c > \"config_file=VALUE\" or -D invocation"? There is some practice for > --opt=VALUE in .po files. Yeah, some of this is becoming quite unwieldy, if we document and mention each spelling variant of each option everywhere. Maybe if the original problem is that the option --config-file is not explicitly in the --help output, let's add it to the --help output?
On Wed, May 15, 2024 at 2:49 AM Peter Eisentraut <peter@eisentraut.org> wrote:
On 15.05.24 04:07, Michael Paquier wrote:
> Not sure that these additions in --help or the docs are necessary.
> The rest looks OK.
>
> - "You must specify the --config-file or -D invocation "
> + "You must specify the --config-file (or equivalent -c) or -D invocation "
>
> How about "You must specify the --config-file, -c
> \"config_file=VALUE\" or -D invocation"? There is some practice for
> --opt=VALUE in .po files.
Yeah, some of this is becoming quite unwieldy, if we document and
mention each spelling variant of each option everywhere.
Where else would this need to be added that was missed? Largely we don't discuss how to bring a setting into effect - rather there is a single reference area that discusses how, and everywhere else just assumes you have read it and goes on to name the setting. On this grounds the proper fix here is probably to not put the how into the message:
"You must specify the config_file option, the -D argument, or the PGDATA environment variable."
And this is only unwieldy because while -D and --config-file both can get to the same result they are not substitutes for each other. Namely if the configuration file is not in the data directory, as is the case on Debian, the choice to use -D is not going to work.
This isn't an error message, I'm not all that worried if we output a wall of text in lieu of pointing the user to the reference page.
Maybe if the original problem is that the option --config-file is not
explicitly in the --help output, let's add it to the --help output?
I'm not opposed to this. Though maybe it is sufficient to do:
--NAME=VALUE (e.g., --config-file='...')
I would do this in addition to removing the explicit how of setting config_file above.
We also don't mention environment variables in the help but that message refers to PGDATA...so the complaint and fix if done on that basis seems a bit selective.
David J.
On Wed, 15 May 2024 at 11:49, Peter Eisentraut <peter@eisentraut.org> wrote: > Yeah, some of this is becoming quite unwieldy, if we document and > mention each spelling variant of each option everywhere. > > Maybe if the original problem is that the option --config-file is not > explicitly in the --help output, let's add it to the --help output? I definitely think it would be useful to list this --config variant in more places, imho it's nicer than the -c variant. Especially in the PGOPTIONS docs it would be useful. People are already using it in the wild and I regressed on support for that in PgBouncer by accident: https://github.com/pgbouncer/pgbouncer/pull/1064
On Wed, May 15, 2024 at 04:47:27PM +0200, Jelte Fennema-Nio wrote: > I definitely think it would be useful to list this --config variant in > more places, imho it's nicer than the -c variant. Especially in the > PGOPTIONS docs it would be useful. People are already using it in the > wild and I regressed on support for that in PgBouncer by accident: > https://github.com/pgbouncer/pgbouncer/pull/1064 Agreed that mentioning the --name variant is useful. I'm not really on board with having one option refer to the other on the pages where both are described, like on --help or the doc page for "postgres". For now, I've applied a patch for the libpq.sgml and config.sgml bits which are improvements of their own. -- Michael
Attachment
Hi, > Agreed that mentioning the --name variant is useful. I'm not really > on board with having one option refer to the other on the pages where > both are described, like on --help or the doc page for "postgres". > > For now, I've applied a patch for the libpq.sgml and config.sgml bits > which are improvements of their own. Thanks, Michael. I propose my original v1 patch for correcting the --help output of 'postgres' too. I agree with the above comments that corresponding changes in v4 became somewhat unwieldy. -- Best regards, Aleksander Alekseev
Attachment
On Thu, May 16, 2024 at 11:57:10AM +0300, Aleksander Alekseev wrote: > I propose my original v1 patch for correcting the --help output of > 'postgres' too. I agree with the above comments that corresponding > changes in v4 became somewhat unwieldy. Thanks for compiling the rest. - printf(_(" --NAME=VALUE set run-time parameter\n")); + printf(_(" --NAME=VALUE set run-time parameter, a shorter form of -c\n")); This part with cross-references in the output is still meh to me, for same reason as for the doc changes I've argued to discard upthread. write_stderr("%s does not know where to find the server configuration file.\n" - "You must specify the --config-file or -D invocation " + "You must specify the --config-file (or equivalent -c) or -D invocation " I can fall behind changing this one, still I'm not sure if this proposal is the optimal choice. Adding this option to --help makes sense when applied to this error message, but that's incomplete in regard with the other GUCs where this concept applies. A different approach would be to do nothing in --help and change the reference of --config-file to -c config_name=VALUE, which would be in line with --help. -- Michael
Attachment
On Thu, May 16, 2024 at 4:11 PM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, May 16, 2024 at 11:57:10AM +0300, Aleksander Alekseev wrote:
> I propose my original v1 patch for correcting the --help output of
> 'postgres' too. I agree with the above comments that corresponding
> changes in v4 became somewhat unwieldy.
Thanks for compiling the rest.
- printf(_(" --NAME=VALUE set run-time parameter\n"));
+ printf(_(" --NAME=VALUE set run-time parameter, a shorter form of -c\n"));
This part with cross-references in the output is still meh to me, for
same reason as for the doc changes I've argued to discard upthread.
I'm fine with leaving these alone. If we did change this I'd want to change both, not just --NAME.
write_stderr("%s does not know where to find the server configuration file.\n"
- "You must specify the --config-file or -D invocation "
+ "You must specify the --config-file (or equivalent -c) or -D invocation "
I would rather just do this:
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 3fb6803998..f827086489 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1828,8 +1828,8 @@ SelectConfigFiles(const char *userDoption, const char *progname)
else
{
write_stderr("%s does not know where to find the server configuration file.\n"
- "You must specify the --config-file or -D invocation "
- "option or set the PGDATA environment variable.\n",
+ "You must specify either the config_file run-time parameter or "
+ "provide the database directory\n",
progname);
return false;
}
index 3fb6803998..f827086489 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1828,8 +1828,8 @@ SelectConfigFiles(const char *userDoption, const char *progname)
else
{
write_stderr("%s does not know where to find the server configuration file.\n"
- "You must specify the --config-file or -D invocation "
- "option or set the PGDATA environment variable.\n",
+ "You must specify either the config_file run-time parameter or "
+ "provide the database directory\n",
progname);
return false;
}
Both "run-time parameter" and "database directory" are words present in the help and the user can find the correct argument if that is the option they want to use. The removal of the mention of the PGDATA environment variable doesn't seem to be a great loss here. This error message doesn't seem to be the correct place to teach the user about all of their options so long as they choose to read the documentation they learn about them there; and we need not prescribe the specific means by which they supply either of those pieces of information - which is the norm. If someone simply runs "postgres" at the command line this message and --help gives them sufficient information to proceed.
David J.
On 2024-May-17, Michael Paquier wrote: > On Thu, May 16, 2024 at 11:57:10AM +0300, Aleksander Alekseev wrote: > > I propose my original v1 patch for correcting the --help output of > > 'postgres' too. I agree with the above comments that corresponding > > changes in v4 became somewhat unwieldy. > > Thanks for compiling the rest. > > - printf(_(" --NAME=VALUE set run-time parameter\n")); > + printf(_(" --NAME=VALUE set run-time parameter, a shorter form of -c\n")); > > This part with cross-references in the output is still meh to me, for > same reason as for the doc changes I've argued to discard upthread. Was the idea considered of moving the --NAME=VALUE line to appear together with -c? We already do that with "-?, --help" and "-V, --version", so I think it's pretty reasonable: Options: -B NBUFFERS number of shared buffers -c NAME=VALUE, --NAME=VALUE set run-time parameter -C NAME print value of run-time parameter, then exit [...] > write_stderr("%s does not know where to find the server configuration file.\n" > - "You must specify the --config-file or -D invocation " > + "You must specify the --config-file (or equivalent -c) or -D invocation " I'd rather change the --help and leave this one alone. About the final paragraph Please read the documentation for the complete list of run-time configuration settings and how to set them on the command line or in the configuration file. I was thinking we could mention that using --describe-config here could help, but the literal output from that is quite ugly and unwieldy, more suitable for machine consumption than humans. Would it be useful to add another output format? Say, a --describe-config=man prints a manpage-style table of options with their descriptions and links to the online manual. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "No nos atrevemos a muchas cosas porque son difíciles, pero son difíciles porque no nos atrevemos a hacerlas" (Séneca)
On Thu, May 16, 2024 at 4:57 AM Aleksander Alekseev <aleksander@timescale.com> wrote: > I propose my original v1 patch for correcting the --help output of > 'postgres' too. I agree with the above comments that corresponding > changes in v4 became somewhat unwieldy. So, who is it exactly that will be confused by the status quo? I mean, let's say you get this error: postgres does not know where to find the server configuration file. You must specify the --config-file or -D invocation option or set the PGDATA environment variable. As I see it, either you know how it works and just made a mistake this time, or you are a beginner. If it's the former, the fact that the error message doesn't mention every possible way of solving the problem does not matter, because you already know how to fix your mistake. If it's the latter, you don't need to know *every* way to fix the problem. You just need to know *one* way to fix the problem. I don't really understand why somebody would look at the existing message and say "gosh, it didn't tell me that I could also use -c!". If you already know that, don't you just ignore the hint and get busy with fixing the problem? If the reason that somebody is upset is because it's not technically true to say that you *must* do one of those things, we could fix that with "You must" -> "You can" or with "You must specify" -> "Specify". The patch you propose is also not terrible or anything, but it goes in the direction of listing every alternative, which will become unpalatable as soon as somebody adds one more way to do it, or maybe it's unpalatable already. Even if we don't do that, I don't see that there's a huge problem here. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes: > If the reason that somebody is upset is because it's not technically > true to say that you *must* do one of those things, we could fix that > with "You must" -> "You can" or with "You must specify" -> "Specify". > The patch you propose is also not terrible or anything, but it goes in > the direction of listing every alternative, which will become > unpalatable as soon as somebody adds one more way to do it, or maybe > it's unpalatable already. I agree that it's not necessary or particularly useful for this hint to be exhaustive. I could get behind your suggestion of s/You must specify/Specify/, but I also think it's fine to do nothing. regards, tom lane
Hi, > Robert Haas <robertmhaas@gmail.com> writes: > > If the reason that somebody is upset is because it's not technically > > true to say that you *must* do one of those things, we could fix that > > with "You must" -> "You can" or with "You must specify" -> "Specify". > > The patch you propose is also not terrible or anything, but it goes in > > the direction of listing every alternative, which will become > > unpalatable as soon as somebody adds one more way to do it, or maybe > > it's unpalatable already. > > I agree that it's not necessary or particularly useful for this hint > to be exhaustive. I could get behind your suggestion of > s/You must specify/Specify/, but I also think it's fine to do nothing. Fair enough, let's leave the help message as is then. I closed the corresponding CF entry. -- Best regards, Aleksander Alekseev
On Mon, May 20, 2024 at 12:20:02PM +0300, Aleksander Alekseev wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> I agree that it's not necessary or particularly useful for this hint >> to be exhaustive. I could get behind your suggestion of >> s/You must specify/Specify/, but I also think it's fine to do nothing. > > Fair enough, let's leave the help message as is then. I closed the > corresponding CF entry. I'm OK to leave this be, as well. -- Michael