Fix unsigned output for signed values in SLRU error reporting

Hi,

On 2022-03-18 22:52:02 +0400, Pavel Borisov wrote:

I've noticed that in SRLU error reporting both signed and unsigned values
are output as %u. I think it is worth correcting this with the very simple
patch attached.

Afaics offset etc can't be negative, so I don't think this really improves
matters. I think there's quite a few other places where we use %u to print
integers that we know aren't negative.

If anything I think we should change the signed integers to unsigned ones. It
might be worth doing that as part of
/messages/by-id/CAJ7c6TPDOYBYrnCAeyndkBktO0WG2xSdYduTF0nxq+vfkmTF5Q@mail.gmail.com

Greetings,

Andres Freund

Afaics offset etc can't be negative, so I don't think this really improves
matters. I think there's quite a few other places where we use %u to print
integers that we know aren't negative.

If anything I think we should change the signed integers to unsigned ones.
It
might be worth doing that as part of

/messages/by-id/CAJ7c6TPDOYBYrnCAeyndkBktO0WG2xSdYduTF0nxq+vfkmTF5Q@mail.gmail.com

That was one of my intentions in the mentioned patch, but I couldn't
confirm that the page number (and offset) in SLRU was used signed not by
purpose. Thank you for confirming this. I will try to replace int to
unsigned where it is relevant in SLRU as part of the mentioned thread.
Though it could be a big change worth a separate patch maybe.

Again thanks!
Pavel

пн, 21 мар. 2022 г. в 16:11, Pavel Borisov <pashkin.elfe@gmail.com>:

Afaics offset etc can't be negative, so I don't think this really improves

matters. I think there's quite a few other places where we use %u to print
integers that we know aren't negative.

If anything I think we should change the signed integers to unsigned
ones. It
might be worth doing that as part of

/messages/by-id/CAJ7c6TPDOYBYrnCAeyndkBktO0WG2xSdYduTF0nxq+vfkmTF5Q@mail.gmail.com

That was one of my intentions in the mentioned patch, but I couldn't
confirm that the page number (and offset) in SLRU was used signed not by
purpose. Thank you for confirming this. I will try to replace int to
unsigned where it is relevant in SLRU as part of the mentioned thread.
Though it could be a big change worth a separate patch maybe.

In the patchset where we're working on making SLRU 64bit [1] we have come

to agreement that:
- signed to unsigned change in SLRU *page* numbering is not needed as
maximum SLRU page number is guaranteed to be much more than 2 times less
than maximum 64-bit XID.
- change of *offset* from int format to the wider one is not needed at all
as multiple of SLRU_PAGES_PER_SEGMENT
and CLOG_XACTS_PER_PAGE (and similar for commit_ts and mxact) is far less
than 2^32 [2]/messages/by-id/20220325.120718.758699124869814269.horikyota.ntt@gmail.com

So the change to printing offset as signed, from this thread, is not going
to be included into SLRU 64-bit thread [1]/messages/by-id/CALT9ZEEf1uywYN+VaRuSwNMGE5=eFOy7ZTwtP2g+W9oJDszqQw@mail.gmail.com.
It's true that offset can not be negative, but printing int value as %u
isn't nice even if it is not supposed to be negative. So I'd propose the
small patch in this thread be applied separately if none has anything
against it.

[1]: /messages/by-id/CALT9ZEEf1uywYN+VaRuSwNMGE5=eFOy7ZTwtP2g+W9oJDszqQw@mail.gmail.com
/messages/by-id/CALT9ZEEf1uywYN+VaRuSwNMGE5=eFOy7ZTwtP2g+W9oJDszqQw@mail.gmail.com
[2]: /messages/by-id/20220325.120718.758699124869814269.horikyota.ntt@gmail.com
/messages/by-id/20220325.120718.758699124869814269.horikyota.ntt@gmail.com

--
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com <http://www.postgrespro.com&gt;

On 25.03.22 11:49, Pavel Borisov wrote:

It's true that offset can not be negative, but printing int value as %u
isn't nice even if it is not supposed to be negative. So I'd propose the
small patch in this thread be applied separately if none has anything
against it.

committed