Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe

On Wed, Aug 7, 2024 at 10:52 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
> On 07.08.24 17:53, Alexander Korotkov wrote:
> > On Wed, Aug 7, 2024 at 12:07 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
> >> On 27.07.24 00:24, Michael Paquier wrote:
> >>> Fix more holes with SLRU code in need of int64 for segment numbers
> >>>
> >>> This is a continuation of 3937cadfd438, taking care of more areas I have
> >>> managed to miss previously.
> >>>
> >>> Reported-by: Noah Misch
> >>> Reviewed-by: Noah Misch
> >>> Discussion: https://postgr.es/m/20240724130059.1f.nmisch@google.com
> >>> Backpatch-through: 17
> >>>
> >>> Branch
> >>> ------
> >>> master
> >>>
> >>> Details
> >>> -------
> >>> https://git.postgresql.org/pg/commitdiff/c9e24573905bef7fc3e4efb02bdb4d0cc8e43c51
> >>
> >> I don't understand this patch. The previous patches that this
> >> references changed various variables to int64 and made adjustments
> >> following from that. But this patch takes variables and function
> >> results that are of type int and casts them to unsigned long long before
> >> printing. I don't see what that accomplishes, and it's not clear based
> >> on just the explanation that this is a continuation of a previous patch
> >> that doesn't do that. Is there a plan to change these things to int64
> >> as well at some point?
> >
> > There is a plan indeed. The patchset [1] should include conversion
> > multixacts to 64-bit (It surely included that in earlier versions, I
> > didn't look the last versions though). I doubt this will be ready for
> > v18. So this commit might be quite preliminary. But I would prefer
> > to leave it there as soon as it has already landed. Opinions?
>
> I think you should change the output formats at the same time as you
> change the variable types. That way the compiler can cross-check this.
> Otherwise, if you later forget to change a variable, these casts will
> hide it. Or if the future patches turn out differently, then we have
> this useless code.
>
> > Links.
> > 1. https://www.postgresql.org/message-id/CAJ7c6TND0bCnwU1SmxTsFewK4XJGBep343vf%2BT%2BGQ-a5S5hC0w%40mail.gmail.com
>
> It looks like the commit I'm talking about here is a subset of v55-0001
> from that thread?

Yes, looks like this.

> So why is some of this being committed now into v17?
> But as I wrote above, I think this approach is a bad idea.

OK, I agree that might look annoying. So, it's better to revert now.
Michael, what do you think?

------
Regards,
Alexander Korotkov
Supabase