From: | Sébastien Lardière <sebastien(at)lardiere(dot)net> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Greg Stark <stark(at)mit(dot)edu>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Timeline ID hexadecimal format |
Date: | 2023-03-07 17:14:58 |
Message-ID: | 6f6ca695-3e2c-8c15-50d7-96049cf87234@lardiere.net |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v5_0001_timelineid_hexadecimal_format.patch | text/x-patch | 3.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Melanie Plageman | 2023-03-07 17:39:29 | Re: add PROCESS_MAIN to VACUUM |
Previous Message | Pavel Borisov | 2023-03-07 16:26:19 | Re: POC: Lock updated tuples in tuple_update() and tuple_delete() |