Re: Add pg_walinspect function with block info columns

On Mon, Mar 20, 2023 at 04:51:19PM -0700, Peter Geoghegan wrote:
> On Mon, Mar 20, 2023 at 4:34 PM Peter Geoghegan <pg@bowt.ie> wrote:
>> I agree. A little redundancy is better when the alternative is fragile
>> code, and I'm pretty sure that that applies here -- there won't be
>> very many duplicated lines, and the final code will be significantly
>> clearer. There can be a comment about keeping GetWALRecordInfo and
>> GetWALBlockInfo in sync.

Well.  Vox populi, Vox dei.  I am deadly outnumbered.

> The new pg_get_wal_block_info outputs columns in an order that doesn't
> seem like the most useful possible order to me. This gives us another
> reason to have separate GetWALRecordInfo and GetWALBlockInfo utility
> functions rather than sharing logic for building output tuples.
>
> Specifically, I think that pg_get_wal_block_info should ouput the
> "primary key" columns first:
>
> reltablespace, reldatabase, relfilenode, blockid, start_lsn, end_lsn

It seems to me that this is up to the one using this SQL?  I am not
sure to follow why this is important.  For the cases you have poked
at, I guess it is, but is strikes me that it is just natural to shape
that to  match the C structures we use for the WAL records
themselves, so the other way around.

> Next comes the columns that duplicate the columns output by
> pg_get_wal_records_info, in the same order as they appear in
> pg_get_wal_records_info. (Obviously this won't include block_ref).

Hmm.  Once you add more record data into pg_get_wal_block_info(), I am
not sure to agree with this statement, actually.  I would think that
the most logical order would be the start_lsn, the end_lsn, the record
information that you want for your quals, the block ID for the block
registered in the record, and finally then the rest of the block
information.  So this puts the record-level data first, and the block
info after.  This would be a bit closer with the order of the WAL
record structure itself, aka XLogRecord and such.

Hence, it seems to me that 0002 has the order pretty much right.
What's the point in adding the description, by the way?  Only
consistency with the other function?  Is that really useful if you
want to apply more quals when retrieving some block data?

Calling GetWALRecordInfo() as part of GetWALBlockInfo() leads to a
rather confusing result, IMO...

@@ -377,6 +385,12 @@ pg_get_wal_block_info(PG_FUNCTION_ARGS)
        while (ReadNextXLogRecord(xlogreader) &&
                   xlogreader->EndRecPtr <= end_lsn)
        {
+               CHECK_FOR_INTERRUPTS();
+
+               /* Get block references, if any, otherwise continue. */
+               if (!XLogRecHasAnyBlockRefs(xlogreader))
+                       continue;

This early shortcut in 0001 is a good idea.
--
Michael