Re: Timeline ID hexadecimal format

On 03.03.23 16:52, Sébastien Lardière wrote:
> On 02/03/2023 09:12, Peter Eisentraut wrote:
>> On 24.02.23 17:27, Sébastien Lardière wrote:
>>> diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
>>> index be05a33205..7e26b51031 100644
>>> --- a/doc/src/sgml/backup.sgml
>>> +++ b/doc/src/sgml/backup.sgml
>>> @@ -1332,7 +1332,8 @@ restore_command = 'cp/mnt/server/archivedir/%f %p'
>>>       you like, add comments to a history file to record your own 
>>> notes about
>>>       how and why this particular timeline was created.  Such 
>>> comments will be
>>>       especially valuable when you have a thicket of different 
>>> timelines as
>>> -    a result of experimentation.
>>> +    a result of experimentation. In both WAL segment file names and 
>>> history files,
>>> +    the timeline ID number is expressed in hexadecimal.
>>>      </para>
>>>        <para>
>>
>> I think here it would be more helpful to show actual examples. Like, 
>> here is a possible file name, this is what the different parts mean.
> 
> So you mean explain the WAL filename and the history filename ? Is it 
> the good place for it ?

Well, your patch says, by the way, the timeline ID in the file is 
hexadecimal.  Then one might ask, what file, what is a timeline, what 
are the other numbers in the file, etc.  It seems very specific in this 
context.  I don't know if the format of these file names is actually 
documented somewhere.

>>> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
>>> index e5c41cc6c6..3b5d041d92 100644
>>> --- a/doc/src/sgml/config.sgml
>>> +++ b/doc/src/sgml/config.sgml
>>> @@ -4110,7 +4110,9 @@ restore_command = 'copy 
>>> "C:\\server\\archivedir\\%f" "%p"'  # Windows
>>>           current when the base backup was taken.  The
>>>           value <literal>latest</literal> recovers
>>>           to the latest timeline found in the archive, which is 
>>> useful in
>>> -        a standby server. <literal>latest</literal> is the default.
>>> +        a standby server. A numerical value expressed in hexadecimal 
>>> must be
>>> +        prefixed with <literal>0x</literal>, for example 
>>> <literal>0x11</literal>.
>>> +        <literal>latest</literal> is the default.
>>>          </para>
>>>            <para>
>>
>> This applies to all configuration parameters, so it doesn't need to be 
>> mentioned explicitly for individual ones.
> 
> Probably, but is there another parameter with the same consequence ?
> 
> worth it to document this point globally ?

It's ok to mention it again.  We do something similar for example at 
unix_socket_permissions.  But maybe with more context, like "If you want 
to specify a timeline ID hexadecimal (for example, if extracted from a 
WAL file name), then prefix it with a 0x".

>>> diff --git a/doc/src/sgml/ref/pg_waldump.sgml 
>>> b/doc/src/sgml/ref/pg_waldump.sgml
>>> index 343f0482a9..4ae8f2ebdd 100644
>>> --- a/doc/src/sgml/ref/pg_waldump.sgml
>>> +++ b/doc/src/sgml/ref/pg_waldump.sgml
>>> @@ -215,7 +215,8 @@ PostgreSQL documentation
>>>          <para>
>>>           Timeline from which to read WAL records. The default is to 
>>> use the
>>>           value in <replaceable>startseg</replaceable>, if that is 
>>> specified; otherwise, the
>>> -        default is 1.
>>> +        default is 1. The value must be expressed in decimal, 
>>> contrary to the hexadecimal
>>> +        value given in WAL segment file names and history files.
>>>          </para>
>>>         </listitem>
>>>        </varlistentry>
>>
>> Maybe this could be fixed instead?
> 
> Indeed, and strtoul is probably a better option than sscanf, don't you 
> think ?

Yeah, the use of sscanf() is kind of weird here.  We have been moving 
the option parsing to use option_parse_int().  Maybe hex support could 
be added there.  Or just use strtoul().