scram_iterations is undocumented GUC_REPORT

Started by Alvaro Herreraabout 2 years ago8 messageshackers
Jump to latest
#1Alvaro Herrera
alvherre@2ndquadrant.com

I noticed while answering a question that commit b577743000cd added the
GUC scram_iterations and marked it GUC_REPORT, but failed to add it to
the PQparameterStatus documentation.

Here's a proposed patch to add it there.

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index d0d5aefadc..13bd82efc6 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2521,7 +2521,8 @@ const char *PQparameterStatus(const PGconn *conn, const char *paramName);
        <varname>DateStyle</varname>,
        <varname>IntervalStyle</varname>,
        <varname>TimeZone</varname>,
-       <varname>integer_datetimes</varname>, and
+       <varname>integer_datetimes</varname>,
+       <varname>scram_iterations</varname>, and
        <varname>standard_conforming_strings</varname>.
        (<varname>server_encoding</varname>, <varname>TimeZone</varname>, and
        <varname>integer_datetimes</varname> were not reported by releases before 8.0;

Further thoughts:

1. that list looks to be in random order. Should we sort it
alphabetically?

2. the notes about the versions in which some parameters started to be
reported, look quite outdated. We don't really care about things not
reported in 8.0 or 8.1 or even 9.0. For all purposes, it seems
perfectly OK to say that these parameters have been reported forever
(i.e. don't mention them in these lists). I think we should remove all
those, except the note about version 14.

3. Should we list scram_iterations as having started to be reported with
version 16? The GUC didn't exist before that; but we could say that if
it's not reported, then the application can assume that the value is
4096 (similar to the wording for standard_conforming_strings).

Thanks

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"Los cuentos de hadas no dan al niño su primera idea sobre los monstruos.
Lo que le dan es su primera idea de la posible derrota del monstruo."
(G. K. Chesterton)

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Alvaro Herrera (#1)
Re: scram_iterations is undocumented GUC_REPORT

On 30 Jan 2024, at 13:36, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

I noticed while answering a question that commit b577743000cd added the
GUC scram_iterations and marked it GUC_REPORT, but failed to add it to
the PQparameterStatus documentation.

Ugh, thanks for fixing!

1. that list looks to be in random order. Should we sort it
alphabetically?

It seems to have some semblance of grouping per GUC functionality, but not
quite. I'm not sure sorting alphabetically will improve the current state
though.

2. the notes about the versions in which some parameters started to be
reported, look quite outdated. We don't really care about things not
reported in 8.0 or 8.1 or even 9.0. For all purposes, it seems
perfectly OK to say that these parameters have been reported forever
(i.e. don't mention them in these lists). I think we should remove all
those, except the note about version 14.

Agreed.

3. Should we list scram_iterations as having started to be reported with
version 16?

Yes, similar to in_hot_standby for v14.

The GUC didn't exist before that; but we could say that if
it's not reported, then the application can assume that the value is
4096 (similar to the wording for standard_conforming_strings).

There is no real practical use for knowing the value if it's not reported,
since there isn't anything the user can do differently knowing that. I would
leave that out to avoid confusion, but I don't have strong feelings if you
think it should be added.

--
Daniel Gustafsson

#3Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Alvaro Herrera (#1)
Re: scram_iterations is undocumented GUC_REPORT

On Tue, 30 Jan 2024 at 13:37, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

I noticed while answering a question that commit b577743000cd added the
GUC scram_iterations and marked it GUC_REPORT, but failed to add it to
the PQparameterStatus documentation.

+1 the improvements your suggesting (although 3 I don't know enough
about to be sure)

One important note though is that this list is tracked in two
different places, so both of these places should be updated:
- doc/src/sgml/protocol.sgml
- doc/src/sgml/libpq.sgml

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#1)
Re: scram_iterations is undocumented GUC_REPORT

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

I noticed while answering a question that commit b577743000cd added the
GUC scram_iterations and marked it GUC_REPORT, but failed to add it to
the PQparameterStatus documentation.

Why is it GUC_REPORT at all? I don't see a strong need for that.
(In particular, the report would be delivered too late to be of any
use in client authentication.)

regards, tom lane

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Jelte Fennema-Nio (#3)
Re: scram_iterations is undocumented GUC_REPORT

On 2024-Jan-30, Jelte Fennema-Nio wrote:

On Tue, 30 Jan 2024 at 13:37, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

I noticed while answering a question that commit b577743000cd added the
GUC scram_iterations and marked it GUC_REPORT, but failed to add it to
the PQparameterStatus documentation.

+1 the improvements your suggesting (although 3 I don't know enough
about to be sure)

One important note though is that this list is tracked in two
different places, so both of these places should be updated:
- doc/src/sgml/protocol.sgml
- doc/src/sgml/libpq.sgml

Ooh, you're right. I propose to turn the list into a
<simplelist type="vert" columns="2">
which looks _much_ nicer to read, as in the attached screenshot of the
PDF.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
<inflex> really, I see PHP as like a strange amalgamation of C, Perl, Shell
<crab> inflex: you know that "amalgam" means "mixture with mercury",
more or less, right?
<crab> i.e., "deadly poison"

Attachments:

v2-0001-Update-PQparameterStatus-and-ParameterStatus-docs.patchtext/x-diff; charset=utf-8Download+40-47
params.pngimage/pngDownload
#6Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#4)
Re: scram_iterations is undocumented GUC_REPORT

On 30 Jan 2024, at 15:36, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

I noticed while answering a question that commit b577743000cd added the
GUC scram_iterations and marked it GUC_REPORT, but failed to add it to
the PQparameterStatus documentation.

Why is it GUC_REPORT at all? I don't see a strong need for that.
(In particular, the report would be delivered too late to be of any
use in client authentication.)

It's needed by PQencryptPasswordConn.

--
Daniel Gustafsson

#7Daniel Gustafsson
daniel@yesql.se
In reply to: Alvaro Herrera (#5)
Re: scram_iterations is undocumented GUC_REPORT

On 30 Jan 2024, at 15:44, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

I propose to turn the list into a
<simplelist type="vert" columns="2">
which looks _much_ nicer to read, as in the attached screenshot of the
PDF.

+1, this reads a lot better.

--
Daniel Gustafsson

#8Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Daniel Gustafsson (#7)
Re: scram_iterations is undocumented GUC_REPORT

On 2024-Feb-07, Daniel Gustafsson wrote:

On 30 Jan 2024, at 15:44, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

I propose to turn the list into a
<simplelist type="vert" columns="2">
which looks _much_ nicer to read, as in the attached screenshot of
the PDF.

+1, this reads a lot better.

Thanks, applied and backpatched to 16.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/