On Tue, Aug 12, 2025 at 07:14:38PM +0900, Fujii Masao wrote:
> On Tue, Aug 12, 2025 at 6:25 PM Japin Li <japi...@hotmail.com> wrote:
> > I believe that the format %X-%X also works with sscanf(). However, to
> > maintain
> > consistency, the format for sscanf() has been updated as well.
>
> Yes.
> Thanks for the patch!
>
> Since we're changing the first "%X" in "%X-%X" to "%08X", the example
> file names in the docs should be updated too. For example:
>
> $ git grep "\.snap" | grep pglogicalinspect.sgml
> doc/src/sgml/pglogicalinspect.sgml:name | 0-40796E18.snap
> doc/src/sgml/pglogicalinspect.sgml:postgres=# SELECT * FROM
> pg_get_logical_snapshot_meta('0-40796E18.snap');
> doc/src/sgml/pglogicalinspect.sgml:name | 0-40796E18.snap
> doc/src/sgml/pglogicalinspect.sgml:name | 0-40796E18.snap
> doc/src/sgml/pglogicalinspect.sgml:postgres=# SELECT * FROM
> pg_get_logical_snapshot_info('0-40796E18.snap');
> doc/src/sgml/pglogicalinspect.sgml:name | 0-40796E18.snap
>
> I also noticed that the regression tests for pg_logicalinspect use file
> names in the old format. This doesn't cause test failures, but should
> we update them to match the new format?
>
> ...
> contrib/pg_logicalinspect/sql/pg_logicalinspect.sql:SELECT
> pg_get_logical_snapshot_info('0/40796E18.snap');
> contrib/pg_logicalinspect/sql/pg_logicalinspect.sql:SELECT
> pg_get_logical_snapshot_info('../snapshots/0-40796E18.snap');
> contrib/pg_logicalinspect/sql/pg_logicalinspect.sql:SELECT
> pg_get_logical_snapshot_meta('0-40796E18.foo.snap');
> contrib/pg_logicalinspect/sql/pg_logicalinspect.sql:SELECT
> pg_get_logical_snapshot_meta('0/40796E18.snap');
> ...
>
Update in v3 patch.
OTOH, I also update reorder buffer spill file path.
$ git grep -E 'xid-.*-lsn.*spill'
src/backend/replication/logical/reorderbuffer.c: snprintf(path, MAXPGPATH,
"%s/%s/xid-%u-lsn-%X-%X.spill",
--
Best regards,
Japin Li
ChengDu WenWu Information Technology Co., LTD.
>From 232e3bfe455769a9aedb7d102986e2d292373cf9 Mon Sep 17 00:00:00 2001
From: Japin Li <japi...@hotmail.com>
Date: Tue, 12 Aug 2025 17:14:46 +0800
Subject: [PATCH v3] Standardize snapshot filename formatting
---
.../expected/pg_logicalinspect.out | 32 +++++++++----------
contrib/pg_logicalinspect/pg_logicalinspect.c | 4 +--
.../sql/pg_logicalinspect.sql | 16 +++++-----
doc/src/sgml/pglogicalinspect.sgml | 14 ++++----
.../replication/logical/reorderbuffer.c | 2 +-
src/backend/replication/logical/snapbuild.c | 8 ++---
6 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/contrib/pg_logicalinspect/expected/pg_logicalinspect.out b/contrib/pg_logicalinspect/expected/pg_logicalinspect.out
index f6c9b625d6b..e1c2233ec43 100644
--- a/contrib/pg_logicalinspect/expected/pg_logicalinspect.out
+++ b/contrib/pg_logicalinspect/expected/pg_logicalinspect.out
@@ -2,16 +2,16 @@ CREATE EXTENSION pg_logicalinspect;
-- ===================================================================
-- Tests for input validation
-- ===================================================================
-SELECT pg_get_logical_snapshot_info('0-40796E18.foo');
-ERROR: invalid snapshot file name "0-40796E18.foo"
-SELECT pg_get_logical_snapshot_info('0--40796E18.snap');
-ERROR: invalid snapshot file name "0--40796E18.snap"
+SELECT pg_get_logical_snapshot_info('00000000-40796E18.foo');
+ERROR: invalid snapshot file name "00000000-40796E18.foo"
+SELECT pg_get_logical_snapshot_info('00000000--40796E18.snap');
+ERROR: invalid snapshot file name "00000000--40796E18.snap"
SELECT pg_get_logical_snapshot_info('-1--40796E18.snap');
ERROR: invalid snapshot file name "-1--40796E18.snap"
-SELECT pg_get_logical_snapshot_info('0/40796E18.foo.snap');
-ERROR: invalid snapshot file name "0/40796E18.foo.snap"
-SELECT pg_get_logical_snapshot_info('0/40796E18.snap');
-ERROR: invalid snapshot file name "0/40796E18.snap"
+SELECT pg_get_logical_snapshot_info('00000000/40796E18.foo.snap');
+ERROR: invalid snapshot file name "00000000/40796E18.foo.snap"
+SELECT pg_get_logical_snapshot_info('00000000/40796E18.snap');
+ERROR: invalid snapshot file name "00000000/40796E18.snap"
SELECT pg_get_logical_snapshot_info('');
ERROR: invalid snapshot file name ""
SELECT pg_get_logical_snapshot_info(NULL);
@@ -22,14 +22,14 @@ SELECT pg_get_logical_snapshot_info(NULL);
SELECT pg_get_logical_snapshot_info('../snapshots');
ERROR: invalid snapshot file name "../snapshots"
-SELECT pg_get_logical_snapshot_info('../snapshots/0-40796E18.snap');
-ERROR: invalid snapshot file name "../snapshots/0-40796E18.snap"
-SELECT pg_get_logical_snapshot_meta('0-40796E18.foo');
-ERROR: invalid snapshot file name "0-40796E18.foo"
-SELECT pg_get_logical_snapshot_meta('0-40796E18.foo.snap');
-ERROR: invalid snapshot file name "0-40796E18.foo.snap"
-SELECT pg_get_logical_snapshot_meta('0/40796E18.snap');
-ERROR: invalid snapshot file name "0/40796E18.snap"
+SELECT pg_get_logical_snapshot_info('../snapshots/00000000-40796E18.snap');
+ERROR: invalid snapshot file name "../snapshots/00000000-40796E18.snap"
+SELECT pg_get_logical_snapshot_meta('00000000-40796E18.foo');
+ERROR: invalid snapshot file name "00000000-40796E18.foo"
+SELECT pg_get_logical_snapshot_meta('00000000-40796E18.foo.snap');
+ERROR: invalid snapshot file name "00000000-40796E18.foo.snap"
+SELECT pg_get_logical_snapshot_meta('00000000/40796E18.snap');
+ERROR: invalid snapshot file name "00000000/40796E18.snap"
SELECT pg_get_logical_snapshot_meta('');
ERROR: invalid snapshot file name ""
SELECT pg_get_logical_snapshot_meta(NULL);
diff --git a/contrib/pg_logicalinspect/pg_logicalinspect.c b/contrib/pg_logicalinspect/pg_logicalinspect.c
index 50e805d3195..fe88857fda1 100644
--- a/contrib/pg_logicalinspect/pg_logicalinspect.c
+++ b/contrib/pg_logicalinspect/pg_logicalinspect.c
@@ -69,7 +69,7 @@ parse_snapshot_filename(const char *filename)
* check including the suffix. The subsequent check validates if the given
* filename has the expected suffix.
*/
- if (sscanf(filename, "%X-%X.snap", &hi, &lo) != 2)
+ if (sscanf(filename, "%08X-%08X.snap", &hi, &lo) != 2)
goto parse_error;
/*
@@ -77,7 +77,7 @@ parse_snapshot_filename(const char *filename)
* to the given filename. This check strictly checks if the given filename
* follows the format of the snapshot filename.
*/
- sprintf(tmpfname, "%X-%X.snap", hi, lo);
+ sprintf(tmpfname, "%08X-%08X.snap", hi, lo);
if (strcmp(tmpfname, filename) != 0)
goto parse_error;
diff --git a/contrib/pg_logicalinspect/sql/pg_logicalinspect.sql b/contrib/pg_logicalinspect/sql/pg_logicalinspect.sql
index 143cf45dc12..e087b0ed31b 100644
--- a/contrib/pg_logicalinspect/sql/pg_logicalinspect.sql
+++ b/contrib/pg_logicalinspect/sql/pg_logicalinspect.sql
@@ -4,19 +4,19 @@ CREATE EXTENSION pg_logicalinspect;
-- Tests for input validation
-- ===================================================================
-SELECT pg_get_logical_snapshot_info('0-40796E18.foo');
-SELECT pg_get_logical_snapshot_info('0--40796E18.snap');
+SELECT pg_get_logical_snapshot_info('00000000-40796E18.foo');
+SELECT pg_get_logical_snapshot_info('00000000--40796E18.snap');
SELECT pg_get_logical_snapshot_info('-1--40796E18.snap');
-SELECT pg_get_logical_snapshot_info('0/40796E18.foo.snap');
-SELECT pg_get_logical_snapshot_info('0/40796E18.snap');
+SELECT pg_get_logical_snapshot_info('00000000/40796E18.foo.snap');
+SELECT pg_get_logical_snapshot_info('00000000/40796E18.snap');
SELECT pg_get_logical_snapshot_info('');
SELECT pg_get_logical_snapshot_info(NULL);
SELECT pg_get_logical_snapshot_info('../snapshots');
-SELECT pg_get_logical_snapshot_info('../snapshots/0-40796E18.snap');
+SELECT pg_get_logical_snapshot_info('../snapshots/00000000-40796E18.snap');
-SELECT pg_get_logical_snapshot_meta('0-40796E18.foo');
-SELECT pg_get_logical_snapshot_meta('0-40796E18.foo.snap');
-SELECT pg_get_logical_snapshot_meta('0/40796E18.snap');
+SELECT pg_get_logical_snapshot_meta('00000000-40796E18.foo');
+SELECT pg_get_logical_snapshot_meta('00000000-40796E18.foo.snap');
+SELECT pg_get_logical_snapshot_meta('00000000/40796E18.snap');
SELECT pg_get_logical_snapshot_meta('');
SELECT pg_get_logical_snapshot_meta(NULL);
SELECT pg_get_logical_snapshot_meta('../snapshots');
diff --git a/doc/src/sgml/pglogicalinspect.sgml b/doc/src/sgml/pglogicalinspect.sgml
index 1c1a9d14e51..54da5bafbb8 100644
--- a/doc/src/sgml/pglogicalinspect.sgml
+++ b/doc/src/sgml/pglogicalinspect.sgml
@@ -44,7 +44,7 @@ name | 0-40796E18.snap
size | 152
modification | 2024-08-14 16:36:32+00
-postgres=# SELECT * FROM pg_get_logical_snapshot_meta('0-40796E18.snap');
+postgres=# SELECT * FROM pg_get_logical_snapshot_meta('00000000-40796E18.snap');
-[ RECORD 1 ]--------
magic | 1369563137
checksum | 1028045905
@@ -52,8 +52,8 @@ version | 6
postgres=# SELECT ss.name, meta.* FROM pg_ls_logicalsnapdir() AS ss,
pg_get_logical_snapshot_meta(ss.name) AS meta;
--[ RECORD 1 ]-------------
-name | 0-40796E18.snap
+-[ RECORD 1 ]--------------------
+name | 00000000-40796E18.snap
magic | 1369563137
checksum | 1028045905
version | 6
@@ -81,11 +81,11 @@ version | 6
<screen>
postgres=# SELECT * FROM pg_ls_logicalsnapdir();
-[ RECORD 1 ]+-----------------------
-name | 0-40796E18.snap
+name | 00000000-40796E18.snap
size | 152
modification | 2024-08-14 16:36:32+00
-postgres=# SELECT * FROM pg_get_logical_snapshot_info('0-40796E18.snap');
+postgres=# SELECT * FROM pg_get_logical_snapshot_info('00000000-40796E18.snap');
-[ RECORD 1 ]------------+-----------
state | consistent
xmin | 751
@@ -104,8 +104,8 @@ catchange_xip | {751,752}
postgres=# SELECT ss.name, info.* FROM pg_ls_logicalsnapdir() AS ss,
pg_get_logical_snapshot_info(ss.name) AS info;
--[ RECORD 1 ]------------+----------------
-name | 0-40796E18.snap
+-[ RECORD 1 ]------------+-----------------------
+name | 00000000-40796E18.snap
state | consistent
xmin | 751
xmax | 751
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 34cf05668ae..ecc15378e61 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -4893,7 +4893,7 @@ ReorderBufferSerializedPath(char *path, ReplicationSlot *slot, TransactionId xid
XLogSegNoOffsetToRecPtr(segno, 0, wal_segment_size, recptr);
- snprintf(path, MAXPGPATH, "%s/%s/xid-%u-lsn-%X-%X.spill",
+ snprintf(path, MAXPGPATH, "%s/%s/xid-%u-lsn-%08X-%08X.spill",
PG_REPLSLOT_DIR,
NameStr(MyReplicationSlot->data.name),
xid, LSN_FORMAT_ARGS(recptr));
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 8532bfd27e5..b13ceae694a 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -1529,7 +1529,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
* unless the user used pg_resetwal or similar. If a user did so, there's
* no hope continuing to decode anyway.
*/
- sprintf(path, "%s/%X-%X.snap",
+ sprintf(path, "%s/%08X-%08X.snap",
PG_LOGICAL_SNAPSHOTS_DIR,
LSN_FORMAT_ARGS(lsn));
@@ -1573,7 +1573,7 @@ SnapBuildSerialize(SnapBuild *builder, XLogRecPtr lsn)
elog(DEBUG1, "serializing snapshot to %s", path);
/* to make sure only we will write to this tempfile, include pid */
- sprintf(tmppath, "%s/%X-%X.snap.%d.tmp",
+ sprintf(tmppath, "%s/%08X-%08X.snap.%d.tmp",
PG_LOGICAL_SNAPSHOTS_DIR,
LSN_FORMAT_ARGS(lsn), MyProcPid);
@@ -1747,7 +1747,7 @@ SnapBuildRestoreSnapshot(SnapBuildOnDisk *ondisk, XLogRecPtr lsn,
Size sz;
char path[MAXPGPATH];
- sprintf(path, "%s/%X-%X.snap",
+ sprintf(path, "%s/%08X-%08X.snap",
PG_LOGICAL_SNAPSHOTS_DIR,
LSN_FORMAT_ARGS(lsn));
@@ -2019,7 +2019,7 @@ CheckPointSnapBuild(void)
* We just log a message if a file doesn't fit the pattern, it's
* probably some editors lock/state file or similar...
*/
- if (sscanf(snap_de->d_name, "%X-%X.snap", &hi, &lo) != 2)
+ if (sscanf(snap_de->d_name, "%08X-%08X.snap", &hi, &lo) != 2)
{
ereport(LOG,
(errmsg("could not parse file name \"%s\"", path)));
--
2.43.0
