Andres Freund <andres@anarazel.de> writes:
> On 2022-11-08 11:28:08 -0500, Tom Lane wrote:
>> Hence, the attached reverts everything 4ab5dae94 did to this function,
>> and most of 0e758ae89 too, and instead makes IsBinaryUpgrade an
>> additional reason to take the immediate-unlink path.
> I wonder if it's worth aiming slightly higher. There's plenty duplicated code
> between the first segment handling and the loop body. Perhaps the if at the
> top just should decide whether to unlink the first segment or not, and we then
> check that in the body of the loop for segno == 0?
I don't care for that. I think the point here is precisely that
we want behavior A for the first segment and behavior B for the
remaining ones, and so I'd prefer to keep the code that does A
and the code that does B distinct. It was a misguided attempt to
share that code that got us into trouble here in the first place.
Moreover, any future changes to either behavior will be that much
harder if we combine the implementations.
regards, tom lane