Date: Fri, 26 Jul 2019 11:30:19 +0200
From: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: pg_walfile_name_offset can return inconsistent values
Message-ID: <20190726113019.0a3b34c7@firost>
In-Reply-To: <20190726.172120.101752680.horikyota.ntt@gmail.com>
References: <20190726.172120.101752680.horikyota.ntt@gmail.com>
Organization: Dalibo
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

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,