Обсуждение: 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 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.

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

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

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

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

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, 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

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

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

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