Thread: pg_walfile_name_offset can return inconsistent values

pg_walfile_name_offset can return inconsistent values

From
Kyotaro Horiguchi
Date:
Hello.

While looking [1], I noticed that pg_walfile_name_offset behaves
somewhat oddly at segment boundary.

select * from (values ('0/16ffffff'), ('0/17000000'), ('0/17000001')) as t(lsn), lateral
pg_walfile_name_offset(lsn::pg_lsn);
    lsn     |        file_name         | file_offset 
------------+--------------------------+-------------
 0/16ffffff | 000000020000000000000016 |    16777215
 0/17000000 | 000000020000000000000016 |           0
 0/17000001 | 000000020000000000000017 |           1


The file names are right as defined, but the return value of the
second line wrong, or at least misleading. It should be (16,
1000000) or (16, FFFFFF). The former is out-of-domain so we would
have no way than choosing the latter. I'm not sure the purpose of
the second output parameter, thus the former might be right
decision.


The function returns the following result after this patch is
applied.

select * from (values ('0/16ffffff'), ('0/17000000'), ('0/17000001')) as t(lsn), lateral
pg_walfile_name_offset(lsn::pg_lsn);
    lsn     |        file_name         | file_offset 
------------+--------------------------+-------------
 0/16ffffff | 000000020000000000000016 |    16777214
 0/17000000 | 000000020000000000000016 |    16777215
 0/17000001 | 000000020000000000000017 |           0


regards.

[1]: https://www.postgresql.org/message-id/20190725193808.1648ddc8@firost

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From ca9e174f53ac4d513163cbe8201746c8d3d2eb62 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Fri, 26 Jul 2019 17:12:24 +0900
Subject: [PATCH] Fix offset of pg_walfile_name_offset.

The function returns the name of the previous segment with the offset
of the given location at segment boundaries. For example, it returns
("...16", 0) returned for '0/17000000'. That is inconsistent, or at
least confusing. This patch changes the function to return the given
LSN - 1 as offset.
---
 src/backend/access/transam/xlogfuncs.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index b35043bf71..79ea0495b4 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -447,6 +447,8 @@ pg_last_wal_replay_lsn(PG_FUNCTION_ARGS)
  * Note that a location exactly at a segment boundary is taken to be in
  * the previous segment.  This is usually the right thing, since the
  * expected usage is to determine which xlog file(s) are ready to archive.
+ * To be consistent to filename, returns the offset one byte before the given
+ * location as offset.
  */
 Datum
 pg_walfile_name_offset(PG_FUNCTION_ARGS)
@@ -480,10 +482,16 @@ pg_walfile_name_offset(PG_FUNCTION_ARGS)
 
     resultTupleDesc = BlessTupleDesc(resultTupleDesc);
 
+    /*
+     * We assume the given location point as one-byte after the location
+     * really wanted.
+     */
+    locationpoint--;
+
     /*
      * xlogfilename
      */
-    XLByteToPrevSeg(locationpoint, xlogsegno, wal_segment_size);
+    XLByteToSeg(locationpoint, xlogsegno, wal_segment_size);
     XLogFileName(xlogfilename, ThisTimeLineID, xlogsegno, wal_segment_size);
 
     values[0] = CStringGetTextDatum(xlogfilename);
-- 
2.16.3


Re: pg_walfile_name_offset can return inconsistent values

From
Jehan-Guillaume de Rorthais
Date:
On Fri, 26 Jul 2019 17:21:20 +0900 (Tokyo Standard Time)
Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:

> Hello.
> 
> While looking [1], I noticed that pg_walfile_name_offset behaves
> somewhat oddly at segment boundary.
> 
> select * from (values ('0/16ffffff'), ('0/17000000'), ('0/17000001')) as
> t(lsn), lateral pg_walfile_name_offset(lsn::pg_lsn);
>     lsn     |        file_name         | file_offset
> ------------+--------------------------+-------------
>  0/16ffffff | 000000020000000000000016 |    16777215
>  0/17000000 | 000000020000000000000016 |           0
>  0/17000001 | 000000020000000000000017 |           1
> 
> 
> The file names are right as defined, but the return value of the
> second line wrong, or at least misleading.

+1
I noticed it as well and put this report on hold while working on my patch.
Thanks for reporting this!

> It should be (16, 1000000) or (16, FFFFFF). The former is out-of-domain so we
> would have no way than choosing the latter. I'm not sure the purpose of
> the second output parameter, thus the former might be right
> decision.
>
> The function returns the following result after this patch is
> applied.
> 
> select * from (values ('0/16ffffff'), ('0/17000000'), ('0/17000001')) as
> t(lsn), lateral pg_walfile_name_offset(lsn::pg_lsn);
>     lsn     |        file_name         | file_offset
> ------------+--------------------------+-------------
>  0/16ffffff | 000000020000000000000016 |    16777214
>  0/17000000 | 000000020000000000000016 |    16777215
>  0/17000001 | 000000020000000000000017 |           0

So you shift the file offset for all LSN by one byte? This could lead to
regression in various tools relying on this function.

Moreover, it looks weird as the LSN doesn't reflect the given offset anymore
(FFFFFF <> 16777214, 000001 <> 0, etc).

Another solution might be to return the same result when for both 0/16ffffff and
0/17000000, but it doesn't feel right either.

So in fact, returning 0x1000000 seems to be the cleaner result to me.

Regards,



Re: pg_walfile_name_offset can return inconsistent values

From
Bruce Momjian
Date:
On Fri, Jul 26, 2019 at 11:30:19AM +0200, Jehan-Guillaume de Rorthais wrote:
> On Fri, 26 Jul 2019 17:21:20 +0900 (Tokyo Standard Time)
> Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
> 
> > Hello.
> > 
> > While looking [1], I noticed that pg_walfile_name_offset behaves
> > somewhat oddly at segment boundary.
> > 
> > select * from (values ('0/16ffffff'), ('0/17000000'), ('0/17000001')) as
> > t(lsn), lateral pg_walfile_name_offset(lsn::pg_lsn);
> >     lsn     |        file_name         | file_offset
> > ------------+--------------------------+-------------
> >  0/16ffffff | 000000020000000000000016 |    16777215
> >  0/17000000 | 000000020000000000000016 |           0
> >  0/17000001 | 000000020000000000000017 |           1
> > 
> > 
> > The file names are right as defined, but the return value of the
> > second line wrong, or at least misleading.
> 
> +1
> I noticed it as well and put this report on hold while working on my patch.
> Thanks for reporting this!
> 
> > It should be (16, 1000000) or (16, FFFFFF). The former is out-of-domain so we
> > would have no way than choosing the latter. I'm not sure the purpose of
> > the second output parameter, thus the former might be right
> > decision.
> >
> > The function returns the following result after this patch is
> > applied.
> > 
> > select * from (values ('0/16ffffff'), ('0/17000000'), ('0/17000001')) as
> > t(lsn), lateral pg_walfile_name_offset(lsn::pg_lsn);
> >     lsn     |        file_name         | file_offset
> > ------------+--------------------------+-------------
> >  0/16ffffff | 000000020000000000000016 |    16777214
> >  0/17000000 | 000000020000000000000016 |    16777215
> >  0/17000001 | 000000020000000000000017 |           0
> 
> So you shift the file offset for all LSN by one byte? This could lead to
> regression in various tools relying on this function.
> 
> Moreover, it looks weird as the LSN doesn't reflect the given offset anymore
> (FFFFFF <> 16777214, 000001 <> 0, etc).
> 
> Another solution might be to return the same result when for both 0/16ffffff and
> 0/17000000, but it doesn't feel right either.
> 
> So in fact, returning 0x1000000 seems to be the cleaner result to me.

I know this bug report is four years old, but it is still a
pg_walfile_name_offset() bug.  Here is the bug:

    SELECT *
    FROM (VALUES ('0/16ffffff'), ('0/17000000'), ('0/17000001')) AS t(lsn), 
         LATERAL pg_walfile_name_offset(lsn::pg_lsn);

        lsn     |        file_name         | file_offset
    ------------+--------------------------+-------------
     0/16ffffff | 000000010000000000000016 |    16777215
-->     0/17000000 | 000000010000000000000016 |           0
     0/17000001 | 000000010000000000000017 |           1

The bug is in the indicated line --- it shows the filename as 00016 but
offset as zero, when clearly the LSN is pointing to 17/0.  The bug is
essentially that the code for pg_walfile_name_offset() uses the exact
offset from the LSN, but uses the file name from the previous byte of
the LSN.

The fix involves deciding what the description or purpose of
pg_walfile_name_offset() means, and adjusting it to be clearer.  The
current documentation says:

    Converts a write-ahead log location to a WAL file name and byte
    offset within that file.

Fix #1:  If we assume write-ahead log location means LSN, it is saying
show the file/offset of the LSN, and that is most clearly:

        lsn     |        file_name         | file_offset
    ------------+--------------------------+-------------
     0/16ffffff | 000000010000000000000016 |    16777215
     0/17000000 | 000000010000000000000017 |           0
     0/17000001 | 000000010000000000000017 |           1

Fix #2:  Now, there are some who have said they want the output to be
the last written WAL byte (the byte before the LSN), not the current
LSN, for archiving purposes.  However, if we do that, we have to update
the docs to clarify it.  Its output would be:

        lsn     |        file_name         | file_offset
    ------------+--------------------------+-------------
     0/16ffffff | 000000010000000000000016 |    16777214
     0/17000000 | 000000010000000000000016 |    16777215
     0/17000001 | 000000010000000000000017 |           0

The email thread also considered having the second row offset be 16777216
(2^24), which is not a valid offset for a file if we assume a zero-based
offset.

Looking further, pg_walfile_name() also returns the filename for the
previous byte:

    SELECT *
    FROM (values ('0/16ffffff'), ('0/17000000'), ('0/17000001')) as t(lsn),
         LATERAL pg_walfile_name_offset(lsn::pg_lsn), LATERAL pg_walfile_name(lsn::pg_lsn);
        lsn     |        file_name         | file_offset |     pg_walfile_name
    ------------+--------------------------+-------------+--------------------------
     0/16ffffff | 000000010000000000000016 |    16777215 | 000000010000000000000016
     0/17000000 | 000000010000000000000016 |           0 | 000000010000000000000016
     0/17000001 | 000000010000000000000017 |           1 | 000000010000000000000017

I have attached fix #1 as offset1.diff and fix #2 as offset2.diff.

I think the most logical fix is #1, but pg_walfile_name() would need to
be modified.  If the previous file/byte offset are what is desired, fix
#2 will need doc changes for both functions.  This probably needs to be
documented as a backward incompatibility for either fix.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Attachment

Re: pg_walfile_name_offset can return inconsistent values

From
Matthias van de Meent
Date:
On Thu, 9 Nov 2023 at 20:22, Bruce Momjian <bruce@momjian.us> wrote:
> I know this bug report is four years old, but it is still a
> pg_walfile_name_offset() bug.  Here is the bug:
>
>         SELECT *
>         FROM (VALUES ('0/16ffffff'), ('0/17000000'), ('0/17000001')) AS t(lsn),
>              LATERAL pg_walfile_name_offset(lsn::pg_lsn);
>
>             lsn     |        file_name         | file_offset
>         ------------+--------------------------+-------------
>          0/16ffffff | 000000010000000000000016 |    16777215
> -->      0/17000000 | 000000010000000000000016 |           0
>          0/17000001 | 000000010000000000000017 |           1
>
> The bug is in the indicated line --- it shows the filename as 00016 but
> offset as zero, when clearly the LSN is pointing to 17/0.  The bug is
> essentially that the code for pg_walfile_name_offset() uses the exact
> offset from the LSN, but uses the file name from the previous byte of
> the LSN.

Yes, that's definitely not a correct result.

> The fix involves deciding what the description or purpose of
> pg_walfile_name_offset() means, and adjusting it to be clearer.  The
> current documentation says:
>
>         Converts a write-ahead log location to a WAL file name and byte
>         offset within that file.
>
> Fix #1:  If we assume write-ahead log location means LSN, it is saying
> show the file/offset of the LSN, and that is most clearly:
>
>             lsn     |        file_name         | file_offset
>         ------------+--------------------------+-------------
>          0/16ffffff | 000000010000000000000016 |    16777215
>          0/17000000 | 000000010000000000000017 |           0
>          0/17000001 | 000000010000000000000017 |           1
>
> Fix #2:  Now, there are some who have said they want the output to be
> the last written WAL byte (the byte before the LSN), not the current
> LSN, for archiving purposes.  However, if we do that, we have to update
> the docs to clarify it.  Its output would be:
>
>             lsn     |        file_name         | file_offset
>         ------------+--------------------------+-------------
>          0/16ffffff | 000000010000000000000016 |    16777214
>          0/17000000 | 000000010000000000000016 |    16777215
>          0/17000001 | 000000010000000000000017 |           0
>
> I have attached fix #1 as offset1.diff and fix #2 as offset2.diff.

I believe you got the references wrong; fix #1 looks like the output
of offset2's changes, and fix #2 looks like the result of offset1's
changes.

Either way, I think fix #1 is most correct (as was attached in
offset2.diff, and quoted verbatim here), because that has no chance of
having surprising underflowing behaviour when you use '0/0'::lsn as
input.

> diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
> index 45a70668b1..e65502d51e 100644
> --- a/src/backend/access/transam/xlogfuncs.c
> +++ b/src/backend/access/transam/xlogfuncs.c
> @@ -414,7 +414,7 @@ pg_walfile_name_offset(PG_FUNCTION_ARGS)
>     /*
>      * xlogfilename
>      */
> -    XLByteToPrevSeg(locationpoint, xlogsegno, wal_segment_size);
> +    XLByteToSeg(locationpoint, xlogsegno, wal_segment_size);
>     XLogFileName(xlogfilename, GetWALInsertionTimeLine(), xlogsegno,
>                  wal_segment_size);

Kind regards,

Matthias van de Meent



Re: pg_walfile_name_offset can return inconsistent values

From
Bruce Momjian
Date:
On Thu, Nov  9, 2023 at 09:49:48PM +0100, Matthias van de Meent wrote:
> > I have attached fix #1 as offset1.diff and fix #2 as offset2.diff.
> 
> I believe you got the references wrong; fix #1 looks like the output
> of offset2's changes, and fix #2 looks like the result of offset1's
> changes.

Sorry, I swaped them around when I realized the order I was posting them
in the email, and got it wrong.

> Either way, I think fix #1 is most correct (as was attached in
> offset2.diff, and quoted verbatim here), because that has no chance of
> having surprising underflowing behaviour when you use '0/0'::lsn as
> input.

Attached is the full patch that changes pg_walfile_name_offset() and
pg_walfile_name().  There is no need for doc changes.  We need to
document this as incompatible in case users are realying on the old
behavior for WAL archiving purposes.  If they want the old behavior they
need to check for an offset of zero and subtract one from the file name.

Can someone check that all other calls to XLByteToPrevSeg() are correct?

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Attachment

Re: pg_walfile_name_offset can return inconsistent values

From
Michael Paquier
Date:
On Thu, Nov 09, 2023 at 04:14:07PM -0500, Bruce Momjian wrote:
> Attached is the full patch that changes pg_walfile_name_offset() and
> pg_walfile_name().  There is no need for doc changes.  We need to
> document this as incompatible in case users are realying on the old
> behavior for WAL archiving purposes.  If they want the old behavior they
> need to check for an offset of zero and subtract one from the file name.

FWIW, I am not really convinced that there is a strong need to
backpatch any of that.  There's a risk that some queries relying on
the old behavior suddenly break after a minor release, and that's
always annoying.  A HEAD-only change seems like a safer bet to me.

> Can someone check that all other calls to XLByteToPrevSeg() are correct?

On a quick check, all the other calls use that for end record LSNs, so
that looks fine.
--
Michael

Attachment

Re: pg_walfile_name_offset can return inconsistent values

From
Bruce Momjian
Date:
On Fri, Nov 10, 2023 at 08:25:35AM +0900, Michael Paquier wrote:
> On Thu, Nov 09, 2023 at 04:14:07PM -0500, Bruce Momjian wrote:
> > Attached is the full patch that changes pg_walfile_name_offset() and
> > pg_walfile_name().  There is no need for doc changes.  We need to
> > document this as incompatible in case users are realying on the old
> > behavior for WAL archiving purposes.  If they want the old behavior they
> > need to check for an offset of zero and subtract one from the file name.
> 
> FWIW, I am not really convinced that there is a strong need to
> backpatch any of that.  There's a risk that some queries relying on
> the old behavior suddenly break after a minor release, and that's
> always annoying.  A HEAD-only change seems like a safer bet to me.

Yes, this cannot be backpatched, clearly.

> > Can someone check that all other calls to XLByteToPrevSeg() are correct?
> 
> On a quick check, all the other calls use that for end record LSNs, so
> that looks fine.

Thank you.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.



Re: pg_walfile_name_offset can return inconsistent values

From
Andres Freund
Date:
Hi,

On 2023-11-09 16:14:07 -0500, Bruce Momjian wrote:
> On Thu, Nov  9, 2023 at 09:49:48PM +0100, Matthias van de Meent wrote:
> > Either way, I think fix #1 is most correct (as was attached in
> > offset2.diff, and quoted verbatim here), because that has no chance of
> > having surprising underflowing behaviour when you use '0/0'::lsn as
> > input.
> 
> Attached is the full patch that changes pg_walfile_name_offset() and
> pg_walfile_name().  There is no need for doc changes.

I think this needs to add tests "documenting" the changed behaviour and
perhaps also for a few other edge cases.  You could e.g. test
  SELECT * FROM pg_walfile_name_offset('0/0')
which today returns bogus values, and which is independent of the wal segment
size.

And with
SELECT setting::int8 AS segment_size FROM pg_settings WHERE name = 'wal_segment_size' \gset
you can test real things too, e.g.:
SELECT segment_number, file_offset FROM pg_walfile_name_offset('0/0'::pg_lsn + :segment_size),
pg_split_walfile_name(file_name);
SELECT segment_number, file_offset FROM pg_walfile_name_offset('0/0'::pg_lsn + :segment_size + 1),
pg_split_walfile_name(file_name);
SELECT segment_number, file_offset = :segment_size - 1 FROM pg_walfile_name_offset('0/0'::pg_lsn + :segment_size - 1),
pg_split_walfile_name(file_name);

Greetings,

Andres Freund



Re: pg_walfile_name_offset can return inconsistent values

From
Michael Paquier
Date:
On Fri, Nov 10, 2023 at 07:59:43PM -0800, Andres Freund wrote:
> I think this needs to add tests "documenting" the changed behaviour and
> perhaps also for a few other edge cases.  You could e.g. test
>   SELECT * FROM pg_walfile_name_offset('0/0')
> which today returns bogus values, and which is independent of the wal segment
> size.

+1.
--
Michael

Attachment

Re: pg_walfile_name_offset can return inconsistent values

From
Bruce Momjian
Date:
On Fri, Nov 10, 2023 at 07:59:43PM -0800, Andres Freund wrote:
> Hi,
> 
> On 2023-11-09 16:14:07 -0500, Bruce Momjian wrote:
> > On Thu, Nov  9, 2023 at 09:49:48PM +0100, Matthias van de Meent wrote:
> > > Either way, I think fix #1 is most correct (as was attached in
> > > offset2.diff, and quoted verbatim here), because that has no chance of
> > > having surprising underflowing behaviour when you use '0/0'::lsn as
> > > input.
> > 
> > Attached is the full patch that changes pg_walfile_name_offset() and
> > pg_walfile_name().  There is no need for doc changes.
> 
> I think this needs to add tests "documenting" the changed behaviour and
> perhaps also for a few other edge cases.  You could e.g. test
>   SELECT * FROM pg_walfile_name_offset('0/0')
> which today returns bogus values, and which is independent of the wal segment
> size.
> 
> And with
> SELECT setting::int8 AS segment_size FROM pg_settings WHERE name = 'wal_segment_size' \gset
> you can test real things too, e.g.:
> SELECT segment_number, file_offset FROM pg_walfile_name_offset('0/0'::pg_lsn + :segment_size),
pg_split_walfile_name(file_name);
> SELECT segment_number, file_offset FROM pg_walfile_name_offset('0/0'::pg_lsn + :segment_size + 1),
pg_split_walfile_name(file_name);
> SELECT segment_number, file_offset = :segment_size - 1 FROM pg_walfile_name_offset('0/0'::pg_lsn + :segment_size -
1),pg_split_walfile_name(file_name);
 

Sure, I have added these tests.

FYI, pg_walfile_name_offset() has this C comment, which I have removed
in this patch;

    * Note that a location exactly at a segment boundary is taken to be in
    * the previous segment.  This is usually the right thing, since the
    * expected usage is to determine which xlog file(s) are ready to archive.

There is also a documentation mention of this behavior:

    When the given write-ahead log location is exactly at a write-ahead log file boundary, both
    these functions return the name of the preceding write-ahead log file.
    This is usually the desired behavior for managing write-ahead log archiving
    behavior, since the preceding file is the last one that currently
    needs to be archived.

After seeing the doc mention, I started digging into the history of this
feature.  It was originally called pg_current_xlogfile_offset() and
proposed in this email thread, which started on 2006-07-31:

    https://www.postgresql.org/message-id/flat/1154384790.3226.21.camel%40localhost.localdomain

In the initial patch by Simon Riggs, there was no "previous segment
file" behavior, just a simple filename/offset calculation.

This was applied on 2006-08-06 with this commit:

    commit 704ddaaa09
    Author: Tom Lane <tgl@sss.pgh.pa.us>
    Date:   Sun Aug 6 03:53:44 2006 +0000
    
        Add support for forcing a switch to a new xlog file; cause such a switch
        to happen automatically during pg_stop_backup().  Add some functions for
        interrogating the current xlog insertion point and for easily extracting
        WAL filenames from the hex WAL locations displayed by pg_stop_backup
        and friends.  Simon Riggs with some editorialization by Tom Lane.

and the email of the commit said:

    https://www.postgresql.org/message-id/18457.1154836638%40sss.pgh.pa.us

    I also made the new user-level functions a bit more orthogonal, so
    that filenames could be extracted from the existing functions like
    pg_stop_backup.

There is later talk about returning last write pointer vs. the current
insert pointer, and having it match what is returned by pg_stop_backup():

    https://www.postgresql.org/message-id/1155124565.2368.95.camel%40localhost.localdomain
    
    Methinks it should be the Write pointer all of the time, since I can't
    think of a valid reason for wanting to know where the Insert pointer is
    *before* we've written to the xlog file. Having it be the Insert pointer
    could lead to some errors.

and I suspect that it was the desire to return the last write pointer
that caused the function to return the previous segment on a boundary
offset. This was intended to simplify log shipping implementations, I
think.

The function eventually was renamed in the xlog-to-wal renaming and moved
from xlog.c to xlogfuncs.c.  This thread in 2022 mentioned the
inconsistency for 0/0, but didn't seem to talk about the inconsistency
of offset vs file name:

    https://www.postgresql.org/message-id/flat/20220204225057.GA1535307%40nathanxps13#d964275c9540d8395e138efc0a75f7e8

and it concluded with:

    Yes, its the deliberate choice of design, or a kind of
    questionable-but-unoverturnable decision.  I think there are many
    external tools conscious of this behavior.

However, with the report about the inconsistency, the attached patch
fixes the behavior and removes the documentation about the odd behavior.
This will need to be mentioned as an incompatibility in the major
version release notes.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Attachment

Re: pg_walfile_name_offset can return inconsistent values

From
Andres Freund
Date:
Hi,

On 2023-11-13 12:14:57 -0500, Bruce Momjian wrote:
> +SELECT *
> +FROM (values ('0/16ffffff'), ('0/17000000'), ('0/17000001')) as t(lsn),
> +     LATERAL pg_walfile_name_offset(lsn::pg_lsn),
> +     LATERAL pg_walfile_name(lsn::pg_lsn);
> +    lsn     |        file_name         | file_offset |     pg_walfile_name      
> +------------+--------------------------+-------------+--------------------------
> + 0/16ffffff | 000000010000000000000016 |    16777215 | 000000010000000000000016
> + 0/17000000 | 000000010000000000000017 |           0 | 000000010000000000000017
> + 0/17000001 | 000000010000000000000017 |           1 | 000000010000000000000017
> +(3 rows)

These would break when testing with a different segment size. Today that's not
the case...

Greetings,

Andres Freund



Re: pg_walfile_name_offset can return inconsistent values

From
Bruce Momjian
Date:
On Mon, Nov 13, 2023 at 09:49:53AM -0800, Andres Freund wrote:
> Hi,
> 
> On 2023-11-13 12:14:57 -0500, Bruce Momjian wrote:
> > +SELECT *
> > +FROM (values ('0/16ffffff'), ('0/17000000'), ('0/17000001')) as t(lsn),
> > +     LATERAL pg_walfile_name_offset(lsn::pg_lsn),
> > +     LATERAL pg_walfile_name(lsn::pg_lsn);
> > +    lsn     |        file_name         | file_offset |     pg_walfile_name      
> > +------------+--------------------------+-------------+--------------------------
> > + 0/16ffffff | 000000010000000000000016 |    16777215 | 000000010000000000000016
> > + 0/17000000 | 000000010000000000000017 |           0 | 000000010000000000000017
> > + 0/17000001 | 000000010000000000000017 |           1 | 000000010000000000000017
> > +(3 rows)
> 
> These would break when testing with a different segment size. Today that's not
> the case...

Okay, test removed in the updated patch.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

Attachment

Re: pg_walfile_name_offset can return inconsistent values

From
Bruce Momjian
Date:
On Mon, Nov 13, 2023 at 02:12:12PM -0500, Bruce Momjian wrote:
> On Mon, Nov 13, 2023 at 09:49:53AM -0800, Andres Freund wrote:
> > Hi,
> > 
> > On 2023-11-13 12:14:57 -0500, Bruce Momjian wrote:
> > > +SELECT *
> > > +FROM (values ('0/16ffffff'), ('0/17000000'), ('0/17000001')) as t(lsn),
> > > +     LATERAL pg_walfile_name_offset(lsn::pg_lsn),
> > > +     LATERAL pg_walfile_name(lsn::pg_lsn);
> > > +    lsn     |        file_name         | file_offset |     pg_walfile_name      
> > > +------------+--------------------------+-------------+--------------------------
> > > + 0/16ffffff | 000000010000000000000016 |    16777215 | 000000010000000000000016
> > > + 0/17000000 | 000000010000000000000017 |           0 | 000000010000000000000017
> > > + 0/17000001 | 000000010000000000000017 |           1 | 000000010000000000000017
> > > +(3 rows)
> > 
> > These would break when testing with a different segment size. Today that's not
> > the case...
> 
> Okay, test removed in the updated patch.

Patch applied to master.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.