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
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index be05a33205..fb86a3fec5 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. The timeline identifier is an integer which
+    is used in hexadecimal format in both WAL segment file names and history files.
    </para>
 
    <para>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e5c41cc6c6..6c0d63b73d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -4110,7 +4110,11 @@ 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, by exemple
+        from WAL filename or history file, must be prefixed with <literal>0x</literal>,
+        for example <literal>0x11</literal> if the WAL filename
+        is <filename>00000011000000A10000004F</filename>.
+        <literal>latest</literal> is the default.
        </para>
 
        <para>
diff --git a/doc/src/sgml/ref/pg_waldump.sgml b/doc/src/sgml/ref/pg_waldump.sgml
index 343f0482a9..d92948c68a 100644
--- a/doc/src/sgml/ref/pg_waldump.sgml
+++ b/doc/src/sgml/ref/pg_waldump.sgml
@@ -215,7 +215,9 @@ 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 can be expressed in decimal or hexadecimal format.
+        The hexadecimal format, given by WAL filename, must be preceded by <literal>0x</literal>,
+        by example <literal>0x11</literal>.
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 44b5c8726e..b29d5223ce 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -770,6 +770,7 @@ usage(void)
 	printf(_("  -R, --relation=T/D/R   only show records that modify blocks in relation T/D/R\n"));
 	printf(_("  -s, --start=RECPTR     start reading at WAL location RECPTR\n"));
 	printf(_("  -t, --timeline=TLI     timeline from which to read WAL records\n"
+			 "                         hexadecimal value, from WAL filename, must be preceded by 0x\n"
 			 "                         (default: 1 or the value used in STARTSEG)\n"));
 	printf(_("  -V, --version          output version information, then exit\n"));
 	printf(_("  -w, --fullpage         only show records with a full page write\n"));
@@ -1007,7 +1008,7 @@ main(int argc, char **argv)
 					private.startptr = (uint64) xlogid << 32 | xrecoff;
 				break;
 			case 't':
-				if (sscanf(optarg, "%u", &private.timeline) != 1)
+				if ( !(private.timeline = strtoul(optarg, NULL, 0)) )
 				{
 					pg_log_error("invalid timeline specification: \"%s\"", optarg);
 					goto bad_argument;

Reply via email to