Re: Timeline ID hexadecimal format

On 06/03/2023 18:04, Peter Eisentraut wrote:
> On 03.03.23 16:52, Sébastien Lardière wrote:
>> On 02/03/2023 09:12, Peter Eisentraut wrote:
>>>
>>> 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.


Well, in the context of this patch, the usage both filename are 
explained juste before, so it seems understandable to me

Timelines are explained in this place : 
https://www.postgresql.org/docs/current/continuous-archiving.html#BACKUP-TIMELINES 
so the patch explains the format there


>
>>>>
>>>
>>> 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".


Ok, I've improved the message


>
>>>
>>> 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().


I've made the change with strtoul

About option_parse_int(), actually, strtoint() is used, do we need a 
option_parse_ul() fonction ?

patch attached,

best regards,


-- 
Sébastien