Re: Hash index: stop registering pin-only bucket pages in WAL (remove REGBUF_NO_CHANGE)

On Tue, Nov 3, 2026 at 9:41 PM Greg Burd <greg(at)burd(dot)me> wrote:
>
> On Sat, 31 Oct 2026 17:02:34 +0530, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > +1. I also think this should be a master-only change helping us to
> > remove the use of REGBUF_NO_CHANGE from hash indexes. Also, the below
> > change to make it work on back-branches appears too hacky to me.
>
> Thanks. Then the back branches stay on 27caf4ca5ae, and this series is
> master only. The #if HASH_WAL_REGISTER_BUCKET_PAGE shim you quoted was
> in Vignesh's REL_18 compatibility patch, and it is not part of this
> series.
>
> v4 is attached, rebased on current master (5681f0cfa5a). The only
> change from v3 is formatting: pgperltidy rewraps a few lines in the new
> TAP test. The C code is the same as in v3.
>
> 0001 Don't register pin-only primary bucket pages in hash WAL records
> 0002 Append the hash primary bucket block number to WAL records only
> when needed (Vignesh; Amit's suggestion)

I think we can merge 0001 and 0002 now. I don't see the need for
additional four bytes in common cases for each WAL record we modify as
part of this patch.

One additional comment:
*
- * Backup Blk 0: primary bucket page
* Backup Blk 1: page from which tuples are deleted
+ *
+ * If block 1 is not the primary bucket page, replay takes a cleanup lock on
+ * bucket_blkno, which is not registered.
*/
typedef struct xl_hash_delete
{
+ BlockNumber bucket_blkno; /* primary bucket page */

After this change, shouldn't Backup Blk 1 be numbered as 0? Because
now 0 doesn't have anything. The same is true for other records that
are modified.

--
With Regards,
Amit Kapila.