Message-ID: <c5b85d9c-5a95-45a7-83c7-5dca0d22fddf@iki.fi>
Date: Tue, 7 Jul 2026 20:08:59 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Bug: mdunlinkfiletag unlinks mainfork seg.0 instead of indicated
 fork+segment
To: Matthias van de Meent <boekewurm+postgres@gmail.com>,
 Thomas Munro <thomas.munro@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Robert Haas <robertmhaas@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>
References: 
 <CAEze2WiWt+9+OnqW1g9rKz0gqxymmt=oe6pKAEDrutdfpDMpTw@mail.gmail.com>
 <CA+hUKGK3bbEsr8cpJXbtLhJChv7_DT2xmFcjPeVUuqQeTYdbSw@mail.gmail.com>
 <CAEze2WjfP95SL_Hsu7GzYXLnQyEsT49zOnNvbY_mBLCFiQra1g@mail.gmail.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: 
 <CAEze2WjfP95SL_Hsu7GzYXLnQyEsT49zOnNvbY_mBLCFiQra1g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/c5b85d9c-5a95-45a7-83c7-5dca0d22fddf%40iki.fi>
Precedence: bulk

On 21/12/2024 03:21, Matthias van de Meent wrote:
> On Sat, 21 Dec 2024 at 01:05, Thomas Munro <thomas.munro@gmail.com> wrote:
>> The patch does not seem unreasonable and I'd like to help tidy this
>> up, but ... hmm, could we also consider going the other way?
>> register_unlink_segment(), mdunlinkfiletag() and the macro that
>> populates md.c's FileTag are internal to md.c, and we don't expect
>> external code to be pushing md.c SYNC_UNLINK_REQUEST requests into the
>> request queue (who would do that and what could the motivation
>> possibly be?)  Doesn't feel like a supported usage to me...  So my
>> question is: what bad thing would happen if we just renamed
>> register_unlink_segment() to register_unlink_tombstone() without
>> fork/seg arguments, to make it clear that it's not really a general
>> purpose unreliable segment unlink mechanism that we want anyone to
>> build more stuff on top of?
> 
> I just noticed I misinterpreted the conditions in mdunlinkfork, so
> that I thought it allowed a user to pass their own forknum into
> register_unlink_segment (as that is called with the user-provided
> forknum). Instead, that branch is only taken when forknum ==
> MAIN_FORKNUM, so I think you might be right that going in the other
> direction is more desirable.
> 
> In that case, something along the lines of the attached would then be
> better - it removes the fork and segno from register_unlink_segment's
> arguments (renamed to register_unlink_tombstone), and Asserts() that
> mdunlinkfiletag only receives a FileTag that contains expected values.

Renaming the function and the assertion makes sense to me. Committed to 
master, thanks!

- Heikki