Re: Remove custom redundant full page write description from GIN

On Tue, Sep 30, 2025 at 05:59:49PM +0500, Andrey Borodin wrote:
> I think this thread should stick to patches 1 and 2. Which look good
> to me, but need few words in commit messages.

With XLogRecGetBlockRefInfo() already doing the same job with much
more information about the block, 0001 makes sense.

+ int32 ntuples;
+ ntuples = ((ginxlogUpdateMeta *) rec)->ntuples;
+ appendStringInfo(buf, "ntuples: %d", ntuples);

In 0002. We have a routine for this type of assignments:
XLogRecGetBlockData(). Let's use it here for clarity.

prevTail and newRightlink are block numbers, meaning %u not %d when
printed.

0001 and 0002 can just be merged together.

> Patches 3, 4 and 5 deserve their own thread about GIN code
> beautification, but may be reviewed here. But perhaps, it's OK for
> them to live here too. They also look correct to me, but, again,
> lack appropriate commit messages.

In 0003, "payload" is local to its block or code in ginRedoInsert(),
still it's true that the extra assignment is useless.

I don't think that I'm going to agree with 0004, this removes a
RelFileLocator that could be useful.

0005 looks OK.

> Patch 6 is significant improvement and for sure must be discussed in
> another thread. The idea seems straightforward to me, but it's WAL
> logging, there might be some unforeseen consequences of any small
> change.

Yep, not sure that I see the full benefit of what you are doing here.
A thread describing problems around the WAL descriptions is not suited
for what you are qualifying as an optimization. Same for 0004. 0003
is small enough that I would not bother with a different thread, now
that you've posted a patch..
--
Michael