On Tue, Aug 12, 2025 at 8:06 PM Japin Li <japinli@hotmail.com> wrote:
>
> On Tue, Aug 12, 2025 at 07:14:38PM +0900, Fujii Masao wrote:
> > On Tue, Aug 12, 2025 at 6:25 PM Japin Li <japinli@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.
Thanks for updating the 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",
Should we also update LOGICAL_REWRITE_FORMAT to use %08X?
That said, the number of places to change is growing, and we're drifting
away from the original issue.... So I started thinking it might be better
to simply applying Shveta's patch to revert the incorrect format change
to fix the problem, and then discussing LSN-based filename standardization
in a separate thread.
Regards,
--
Fujii Masao
