Re: Cleaning up nbtree after logical decoding on standby work

On Thu, May 25, 2023 at 06:50:31PM -0700, Peter Geoghegan wrote:
> It's possible that Bertand would have done it this way to begin with
> were it not for the admittedly pretty bad nbtree convention around
> P_NEW. It would be nice to get rid of P_NEW in the near future, too --
> I gather that there was discussion of that in the context of recent
> work in this area.

Nice cleanup overall.

+ if (XLogStandbyInfoActive() && RelationNeedsWAL(rel))
{
- /* Okay to use page. Initialize and return it. */
- _bt_pageinit(page, BufferGetPageSize(buf));
- return buf;
+ safexid = BTPageGetDeleteXid(page);
+ isCatalogRel = RelationIsAccessibleInLogicalDecoding(heaprel);
+ _bt_log_reuse_page(rel, blkno, safexid, isCatalogRel);

There is only one caller of _bt_log_reuse_page(), so assigning a
boolean rather than the heap relation is a bit strange to me. I think
that calling RelationIsAccessibleInLogicalDecoding() within
_bt_log_reuse_page() where xlrec_reuse is filled with its data is much
more natural, like HEAD. One argument in favor of HEAD is that it is
not possible to pass down a wrong value for isCatalogRel, but your
patch would make that possible.
--
Michael