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