Feedback-ID: ie0a040ee:Fastmail
Message-ID: <fb97d0ae-a0bc-411d-8a87-f84e7e146488@eisentraut.org>
Date: Tue, 16 Dec 2025 13:18:25 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM
 on-access)
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>,
 Andrey Borodin <x4mmm@yandex-team.ru>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Heikki Linnakangas <hlinnaka@iki.fi>, Kirill Reshke <reshkekirill@gmail.com>
References: 
 <CAAKRu_YX0NP_yhXvPnvDRjVxxprsRBM-_MZzAJskfMydMQ=ETA@mail.gmail.com>
 <CA+TgmoYgCs=SEsohP6Z6R3KKsGaqFqvqxH8vQ_-nY4t+7rK8jg@mail.gmail.com>
 <CAAKRu_ZP-3=SaZykpwDBMJOdUKyW3Wm5JZfPFRR3L5Ac8ouq4w@mail.gmail.com>
 <CAAKRu_bgkOQqu3K5n4YLRsNBZqJ9Rjg80ROqgKSr2UGz4b5hUg@mail.gmail.com>
 <2wk7jo4m4qwh5sn33pfgerdjfujebbccsmmlownybddbh6nawl@mdyyqpqzxjek>
 <CAAKRu_YR-COJ9aGnMUQqt5yoWmUBjikqrd4jGNZYouHaXpis9g@mail.gmail.com>
 <CALdSSPhiCwJwWwgJP1NmqRmnp9RS2tGOBY0gQrfLCbB+OS5_KQ@mail.gmail.com>
 <CAAKRu_YS+Ocm=OzMaZnG4egFiE9v4VYfZ25DXd6jbwegqmGYbQ@mail.gmail.com>
 <CAAKRu_ZGZSqhGt-RcmmfiSheC+1fjQdxy6_+oM-1jMn8hyVptQ@mail.gmail.com>
 <CALdSSPg+B8RTzTXhJvCcKJBqgzhPZkq0E2oqxQdv74ZNZOMVzg@mail.gmail.com>
 <CAAKRu_Zha7HcdQBv8tTtQrcry5332J6kHnOc1X=TT03LzUXDow@mail.gmail.com>
 <CAAKRu_amF00f2T_H8N6pbbe75C22EeX1OqA=svpj8LNO1sdUuw@mail.gmail.com>
 <bfc015b8-eaea-423d-9a09-1d39c4fe9947@eisentraut.org>
 <CAAKRu_bsDnYW-8PLncPvqb5ndcUriCL9pBdimNkYWTQV4sxO-A@mail.gmail.com>
Content-Language: en-US
From: Peter Eisentraut <peter@eisentraut.org>
In-Reply-To: 
 <CAAKRu_bsDnYW-8PLncPvqb5ndcUriCL9pBdimNkYWTQV4sxO-A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/fb97d0ae-a0bc-411d-8a87-f84e7e146488%40eisentraut.org>
Precedence: bulk

On 15.12.25 22:05, Melanie Plageman wrote:
> On Sat, Dec 13, 2025 at 8:59 AM Peter Eisentraut <peter@eisentraut.org> wrote:
>>
>> On 20.11.25 18:19, Melanie Plageman wrote:
>>> +     prstate->deadoffsets = (OffsetNumber *) presult->deadoffsets;
>>
>> In your patch
>> v22-0001-Split-heap_page_prune_and_freeze-into-helpers.patch, the
>> assignment above casts away the const qualification of the function
>> argument presult:
> 
> Yea, this code (prune_freeze_setup() with a const-qualified
> PruneFreezeResult parameter) is actually already in master -- not just
> in this patchset.
> 
>> +static void
>> +prune_freeze_setup(PruneFreezeParams *params,
>> +                                  TransactionId new_relfrozen_xid,
>> +                                  MultiXactId new_relmin_mxid,
>> +                                  const PruneFreezeResult *presult,
>> +                                  PruneState *prstate)
>>
>> (The cast is otherwise unnecessary, since the underlying type is the
>> same on both sides.)
>>
>> Since prstate->deadoffsets is in fact later modified, this makes the
>> original const qualification invalid.
> 
> I didn't realize I was misusing const here. What I meant to indicate
> by defining the prune_freeze_setup() parameter, as const, is that the
> PruneFreezeResult wouldn't be modified by prune_freeze_setup(). I did
> not mean to indicate that no members of PruneFreezeResult would ever
> be modified.

I'm not sure there is a difference between these two statements.  The 
struct won't be modified is the same as none of its fields will be modified.

> deadoffsets is not modified in prune_freeze_setup(). So,
> are you saying that I can't define a parameter as const if even the
> caller modifies it?

You are not modifying deadoffsets in prune_freeze_setup(), but you are 
assigning its address to a pointer variable that is not const-qualified, 
and so it could be used to modify it later on.

A caller to prune_freeze_setup() that sees the signature const 
PruneFreezeResult *presult could pass a pointer to a PruneFreezeResult 
object that is notionally in read-only memory.  But through the 
non-const-qualified pointer you could later modify the pointed-to 
memory, which would be invalid.  The point of propagating the qualifiers 
is to prevent that at compile time.

If what you want is something like, "prune_freeze_setup() does not 
change any of the fields of what presult points to, but it does record a 
pointer to one of its fields with the intention of modifying it later 
after prune_freeze_setup() is finished", then I think C cannot represent 
that with this API.

Here is a simplified example:

#include <stdlib.h>

// corresponds to PruneFreezeResult
struct foo
{
	int offsets[5];
};

// corresponds to PruneState
struct bar
{
	int *offsets;
};

static void setup(const struct foo *f)
{
	struct bar *b = malloc(sizeof(struct bar));

	b->offsets = f->offsets;  // warning
}

This produces a warning:

test.c:20:20: warning: assignment discards 'const' qualifier from 
pointer target type

The reason is that what "f" points to is const, which means that all its 
fields are const.  The fix is to remove the const from the function 
argument declaration.

One of the possible sources of confusion here is that one struct uses an 
array and the other a pointer, and these sometimes behave similarly and 
sometimes not.