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