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.kyot...@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

Reply via email to