Re: Improve documentation regarding custom settings, placeholders, and the administrative functions - Mailing list pgsql-hackers

On 19/10/2024 23:11, David G. Johnston wrote:
> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
> index 934ef5e469..4478d0aa91 100644
> --- a/doc/src/sgml/config.sgml
> +++ b/doc/src/sgml/config.sgml
> @@ -23,7 +23,7 @@
> 
>       <para>
>        All parameter names are case-insensitive. Every parameter takes a
> -     value of one of five types: boolean, string, integer, floating point,
> +     non-null value of one of five types: boolean, string, integer, 
> floating point,
>        or enumerated (enum).  The type determines the syntax for setting the
>        parameter:
>       </para>

Feels a bit obtuse to be honest. Who would've thought that they can be 
NULL? They're not regular SQL datatypes, after all.

> @@ -11350,14 +11350,20 @@ dynamic_library_path = 'C:\tools\postgresql;H: 
> \my_project\lib;$libdir'
>       <para>
>        Because custom options may need to be set in processes that have not
>        loaded the relevant extension module, <productname>PostgreSQL</ 
> productname>
> -     will accept a setting for any two-part parameter name.  Such variables
> -     are treated as placeholders and have no function until the module that
> -     defines them is loaded. When an extension module is loaded, it 
> will add
> +     will accept a setting for any two-part parameter name.
> +     When an extension module is loaded, it will add
>        its variable definitions and convert any placeholder values 
> according to
>        those definitions.  If there are any unrecognized placeholders
>        that begin with its extension name, warnings are issued and those
>        placeholders are removed.
>       </para>

-1, this completely removes the explanation of what a placeholder 
variable is, but the term placeholder is still used in the text that 
follows.

> +
> +    <para>
> +     If a placeholder is created in a session it will exist for the
> +     lifetime of the session unless removed by an extension.
> +     Placeholders have a string data type with a reset value of the 
> empty string.
> +    </para>

Placeholders can be created in postgresql.conf too, so this emphasis on 
"created in a session" feels a bit off.

> diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
> index ad663c94d7..605bf533ee 100644
> --- a/doc/src/sgml/func.sgml
> +++ b/doc/src/sgml/func.sgml
> @@ -28157,7 +28157,7 @@ acl      | {postgres=arwdDxtm/postgres,foo=r/ 
> postgres}
>           <returnvalue>text</returnvalue>
>          </para>
>          <para>
> -        Returns the current value of the
> +        Returns the current non-null value of the
>           setting <parameter>setting_name</parameter>.  If there is no such
>           setting, <function>current_setting</function> throws an error
>           unless <parameter>missing_ok</parameter> is supplied and

Feels a bit cavalier to mention the "non-null" like this. I understand 
that you wanted to make it clear that when current_setting() returns 
NULL, it means that the setting did not exist and you used 
missing_ok=true. But to deduce that, you still need to know that the 
value cannot be NULL. Otherwise you're left wondering what the function 
would return for NULL values, since this only describes what it returns 
for non-null values.

> @@ -28191,6 +28191,17 @@ acl      | {postgres=arwdDxtm/postgres,foo=r/ 
> postgres}
>           use <literal>false</literal> instead. This function corresponds to
>           the SQL command <xref linkend="sql-set"/>.
>          </para>
> +       <para>
> +        <function>set_config</function> accepts the NULL value for
> +        <parameter>new_value</parameter>, but as settings cannot be 
> null this input
> +        is interpreted as a request to set the setting to its default 
> value.
> +       </para>

+1 on this part. Committed this paragraph so that we can make some 
incremental progress.

> +       <para>
> +        If <parameter>setting_name</parameter> does not already exist
> +        <function>set_config</function> throws an error unless the 
> identifier is a valid
> +        <link linkend="runtime-config-custom">custom option</link> 
> name, in which it
> +        creates a placeholder with the empty string as its old value.
> +       </para>
>          <para>
>           <literal>set_config('log_statement_stats', 'off', false)</literal>
>           <returnvalue>off</returnvalue>

I was sold on this at first, but then I realized that the SET 
documentation doesn't mention placeholder variables either. I think it 
would be better to document them for SET, there's more room for 
discussion there than in this table. The set_config() documentation 
already mentions that it corresponds to the SET command.

-- 
Heikki Linnakangas
Neon (https://neon.tech)