Re: Add pg_walinspect function with block info columns - Mailing list pgsql-hackers

On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote:
> Please see the attached v4 patch set addressing all the review comments.

-   desc = GetRmgr(XLogRecGetRmid(record));
-   id = desc.rm_identify(XLogRecGetInfo(record));
-
-   if (id == NULL)
-       id = psprintf("UNKNOWN (%x)", XLogRecGetInfo(record) & ~XLR_INFO_MASK);
-
-   initStringInfo(&rec_desc);
-   desc.rm_desc(&rec_desc, record);
-
-   /* Block references. */
-   initStringInfo(&rec_blk_ref);
-   XLogRecGetBlockRefInfo(record, false, true, &rec_blk_ref, &fpi_len);
-
-   main_data_len = XLogRecGetDataLen(record);

I don't see any need to move this block of code?  This leads to
unnecessary diffs, potentially making backpatch a bit harder.  Either
way is not a big deal, still..  Except for this bit, 0001 looks fine
by me.

        OUT reltablespace oid,
        OUT reldatabase oid,
        OUT relfilenode oid,
        OUT relblocknumber int8,
+       OUT blockid int2,
+    OUT start_lsn pg_lsn,
+    OUT end_lsn pg_lsn,
+    OUT prev_lsn pg_lsn,

I'd still put the LSN data before the three OIDs for consistency with
the structures, though my opinion does not seem to count much..
--
Michael