pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Peter Eisentraut <peter@eisentraut.org>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
Date: Wed, 7 Aug 2024 21:52:40 +0200
Message-ID: <1b24364d-045e-4392-b98d-d335584cb42a@eisentraut.org> (raw)
In-Reply-To: <CAPpHfduYFXoJrzssZijKTov5wM+Ru3uvwYPqkVAUx3Dc6h_czg@mail.gmail.com>
References: <E1sXTMM-001MoF-2q@gemulon.postgresql.org>
	<92fe572d-638e-4162-aef6-1c42a2936f25@eisentraut.org>
	<CAPpHfduYFXoJrzssZijKTov5wM+Ru3uvwYPqkVAUx3Dc6h_czg@mail.gmail.com>

On 07.08.24 17:53, Alexander Korotkov wrote:
> On Wed, Aug 7, 2024 at 12:07 PM Peter Eisentraut <peter@eisentraut.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...

It looks like the commit I'm talking about here is a subset of v55-0001 
from that thread?  So why is some of this being committed now into v17? 
But as I wrote above, I think this approach is a bad idea.






view thread (18+ messages)  latest in thread

Message-ID: <1b24364d-045e-4392-b98d-d335584cb42a@eisentraut.org>
Permalink:  ../1b24364d-045e-4392-b98d-d335584cb42a@eisentraut.org/
Also on:    postgresql.org/message-id/1b24364d-045e-4392-b98d-d335584cb42a@eisentraut.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: peter@eisentraut.org, aekorotkov@gmail.com, michael@paquier.xyz, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Fix more holes with SLRU code in need of int64 for segment numbe
  In-Reply-To: <1b24364d-045e-4392-b98d-d335584cb42a@eisentraut.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox