Thanks for the various perspectives and feedback.

Attached v2 has additional info for xl_btree_vacuum and xl_btree_delete.

I've quoted various emails by various senders below and replied.

On Fri, Jan 27, 2023 at 3:02 PM Robert Haas <robertmh...@gmail.com> wrote:
>
> On Fri, Jan 27, 2023 at 12:24 PM Melanie Plageman
> <melanieplage...@gmail.com> wrote:
> > I believe I have addressed this in the attached patch.
>
> I'm not sure what's best in terms of formatting details but I
> definitely like the idea of making pg_waldump show more details. I'd
> even like to have a way to extract the tuple data, when it's
> operations on tuples and we have those tuples in the payload. That'd
> be a lot more verbose than what you are doing here, though, and I'm
> not saying you should go do it right now or anything like that.

If I'm not mistaken, this would be quite difficult without changing
rm_desc to return some kind of self-describing data type.

On Tue, Jan 31, 2023 at 4:52 PM Peter Geoghegan <p...@bowt.ie> wrote:
> On Fri, Jan 27, 2023 at 9:24 AM Melanie Plageman
> <melanieplage...@gmail.com> wrote:
> > I started documenting it in the rmgr_utils.h header file in a comment,
> > however it may be worth a README?
> >
> > I haven't polished this description of the format (or added examples,
> > etc) or used it in the btree-related functions because I assume the
> > format and helper function API will need more discussion.
>
> I think that standardization is good, but ISTM that we need clarity on
> what the scope is -- what is *not* being standardized? It may or may
> not be useful to call the end result an API. Or it may not make sense
> to do so in the first committed version, even though we may ultimately
> end up as something that deserves to be called an API. The obligation
> to not break tools that are scraping the output in whatever way seems
> kind of onerous right now -- just not having any gratuitous
> inconsistencies (e.g., fixing totally inconsistent punctuation, making
> the names for fields across WAL records consistent when they serve
> exactly the same purpose) would be a big improvement.

So, we can scrap any README or big comment, but are there other changes
to the code or structure you think would avoid it being seen as an
API?

> As I mentioned in passing already, I actually don't think that the
> B-Tree WAL records are all that special, as far as this stuff goes.
> For example, the DELETE Btree record type is very similar to heapam's
> PRUNE record type, and practically identical to Btree's VACUUM record
> type. All of these record types use the same basic conventions, like a
> snapshotConflictHorizon field for recovery conflicts (which is
> generated in a very similar way during original execution, and
> processed in precisely the same way during REDO), and arrays of page
> offset numbers sorted in ascending order.
>
> There are some remaining details where things from an index AM WAL
> record aren't directly analogous (or pretty much identical) to some
> other heapam WAL records, such as the way that the DELETE Btree record
> type deals with deleting a subset of TIDs from a posting list index
> tuple (generated by B-Tree deduplication). But even these exceptions
> don't require all that much discussion. You could either choose to
> only display the array of deleted index tuple page offset numbers, as
> well as the similar array of "updated" index tuple page offset numbers
> from xl_btree_delete, in which case you just display two arrays of
> page offset numbers, in the same standard way. You may or may not want
> to also show each individual xl_btree_update entry -- doing so would
> be kinda like showing the details of individual freeze plans, except
> that you'd probably display something very similar to the page offset
> number display here too (even though these aren't page offset numbers,
> they're 0-based offsets into the posting list's item pointer data
> array).

I have added detail to xl_btree_delete and xl_btree_vacuum. I have added
the updated/deleted target offset numbers and the updated tuples
metadata.

I wondered if there was any reason to do xl_btree_dedup deduplication
intervals.

> BTW, there is also a tendency for non-btree index AM WAL records to be
> fairly similar or even near-identical to the B-Tree WAL records. While
> Hash indexes are very different to B-Tree indexes at a high level, it
> is nevertheless the case that xl_hash_vacuum_one_page is directly
> based on xl_btree_delete/xl_btree_vacuum, and that xl_hash_insert is
> directly based on xl_btree_insert. There are some other WAL record
> types that are completely different across hash and B-Tree, which is a
> reflection of the fact that the index grows using a totally different
> approach in each AM -- but that doesn't seem like something that
> throws up any roadblocks for you (these can all be displayed as simple
> structs anyway).

I chose not to take on any other index types until I saw if this was viable.

> > Perhaps there should also be example output of the offset arrays in
> > pgwalinspect docs?
>
> That would definitely make sense.

I wanted to include at least a minimal example for those following along
with this thread that would cause creation of one of the record types
which I have enhanced, but I had a little trouble making a reliable
example.

Below is my strategy for getting a Heap PRUNE record with redirects, but
it occasionally doesn't end up working and I wasn't sure why (I can do
more investigation if we think that having some kind of test for this is
useful).

CREATE EXTENSION pg_walinspect;
DROP TABLE IF EXISTS lsns;
CREATE TABLE lsns(name TEXT, lsn pg_lsn);

DROP TABLE IF EXISTS baz;
create table baz(a int, b int) with (autovacuum_enabled=false);
insert into baz select i, i % 3 from generate_series(1,100)i;

update baz set b = 0 where b = 1;
update baz set b = 7 where b = 0;
INSERT INTO lsns VALUES('start_lsn', (SELECT pg_current_wal_lsn()));
vacuum baz;
select count(*) from baz;
INSERT INTO lsns VALUES('end_lsn', (SELECT pg_current_wal_lsn()));
SELECT * FROM pg_get_wal_records_info((select lsn from lsns where name
= 'start_lsn'),
        (select lsn from lsns where name = 'end_lsn'))
    WHERE record_type LIKE 'PRUNE%' AND resource_manager = 'Heap2' LIMIT 1;

> > Personally, I like having the infomasks for the freeze plans. If we
> > someday have a more structured input to rmgr_desc, we could then easily
> > have them in their own column and use functions like
> > heap_tuple_infomask_flags() on them.
>
> I agree, in general, though long term the best approach is one that
> has a configurable level of verbosity, with some kind of roughly
> uniform definition of verbosity (kinda like DEBUG1 - DEBUG5, though
> probably with only 2 or 3 distinct levels).

Given this comment and Robert's concern quoted below, I am wondering if
the consensus is that a lack of verbosity control is a dealbreaker for
adding offsets or not.

On Wed, Feb 1, 2023 at 12:52 PM Robert Haas <robertmh...@gmail.com> wrote:
> On Wed, Feb 1, 2023 at 12:47 PM Peter Geoghegan <p...@bowt.ie> wrote:
> > On Wed, Feb 1, 2023 at 5:20 AM Robert Haas <robertmh...@gmail.com> wrote:
> > > If we're dumping a lot of details out of each WAL record, we might
> > > want to switch to a multi-line format of some kind. No one enjoys a
> > > 460-character wide line, let alone 46000.
> >
> > I generally prefer it when I can use psql without using expanded table
> > format mode, and without having to use a pager. Of course that isn't
> > always possible, but it often is. I just don't think that that's going
> > to become feasible with pg_walinspect queries any time soon, since it
> > really requires a comprehensive strategy to deal with the issue of
> > verbosity.
>
> Well, if we're thinking of making the output a lot more verbose, it
> seems like we should at least do a bit of brainstorming about what
> that strategy could be.

In terms of strategies for controlling output verbosity, it seems
difficult to do without changing the rmgrdesc function signature. Unless
you are thinking of trying to reparse the rmgrdesc string output on the
pg_walinspect/pg_waldump side?

I think if there was a more structured output of rmgrdesc, then this
would also solve the verbosity level problem. Consumers could decide on
their verbosity level -- in various pg_walinspect function outputs, that
would probably just be column selection. For pg_waldump, I imagine that
some kind of parameter or flag would work.

Unless you are suggesting that we add a verbosity parameter to the
rmgrdesc function API now?

On Thu, Mar 2, 2023 at 3:17 AM Peter Eisentraut
<peter.eisentr...@enterprisedb.com> wrote:
> On 01.03.23 17:11, Melanie Plageman wrote:
> > diff --git a/contrib/pg_walinspect/pg_walinspect--1.0.sql 
> > b/contrib/pg_walinspect/pg_walinspect--1.0.sql
> > index 08b3dd5556..eb8ff82dd8 100644
> > --- a/contrib/pg_walinspect/pg_walinspect--1.0.sql
> > +++ b/contrib/pg_walinspect/pg_walinspect--1.0.sql
> > @@ -17,7 +17,7 @@ CREATE FUNCTION pg_get_wal_record_info(IN in_lsn pg_lsn,
> >       OUT main_data_length int4,
> >       OUT fpi_length int4,
> >       OUT description text,
> > -    OUT block_ref text
> > +    OUT block_ref int4[][]
> >   )
> >   AS 'MODULE_PATHNAME', 'pg_get_wal_record_info'
> >   LANGUAGE C STRICT PARALLEL SAFE;
>
> A change like this would require a new extension version and an upgrade
> script.
>
> I suppose it's ok to postpone that work while the actual meat of the
> patch is still being worked out, but I figured I'd mention it in case it
> wasn't considered yet.

Thanks for letting me know. This pg_walinspect patch ended up being
discussed over in [1].

- Melanie

[1] 
https://www.postgresql.org/message-id/flat/CAAKRu_bORebdZmcV8V4cZBzU8M_C6tDDdbiPhCZ6i-iuSXW9TA%40mail.gmail.com
From 068f0caaf1188be2dcc979efc54979260ed53d5b Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Wed, 25 Jan 2023 08:38:38 -0500
Subject: [PATCH v2 1/2] Add rmgr_desc utilities

- Begin to standardize format of rmgr_desc output, adding a new
  rmgr_utils file with comments about expected format.
- Add helper which prints arrays in a standard format and use it in
  heapdesc to print out offset arrays, relid arrays, freeze plan arrays,
  and relid arrays
---
 src/backend/access/rmgrdesc/Makefile         |   1 +
 src/backend/access/rmgrdesc/heapdesc.c       | 153 +++++++++++++++----
 src/backend/access/rmgrdesc/meson.build      |   1 +
 src/backend/access/rmgrdesc/rmgrdesc_utils.c |  88 +++++++++++
 src/bin/pg_waldump/Makefile                  |   2 +-
 src/include/access/heapam_xlog.h             |   1 +
 src/include/access/rmgrdesc_utils.h          |  33 ++++
 7 files changed, 250 insertions(+), 29 deletions(-)
 create mode 100644 src/backend/access/rmgrdesc/rmgrdesc_utils.c
 create mode 100644 src/include/access/rmgrdesc_utils.h

diff --git a/src/backend/access/rmgrdesc/Makefile b/src/backend/access/rmgrdesc/Makefile
index f88d72fd86..cd95eec37f 100644
--- a/src/backend/access/rmgrdesc/Makefile
+++ b/src/backend/access/rmgrdesc/Makefile
@@ -23,6 +23,7 @@ OBJS = \
 	nbtdesc.o \
 	relmapdesc.o \
 	replorigindesc.o \
+	rmgrdesc_utils.o \
 	seqdesc.o \
 	smgrdesc.o \
 	spgdesc.o \
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index 628f7e8215..38cff5ac26 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -15,22 +15,56 @@
 #include "postgres.h"
 
 #include "access/heapam_xlog.h"
+#include "access/rmgrdesc_utils.h"
+
 
 static void
 out_infobits(StringInfo buf, uint8 infobits)
 {
+	if ((infobits & XLHL_XMAX_IS_MULTI) == 0 &&
+		(infobits & XLHL_XMAX_LOCK_ONLY) == 0 &&
+		(infobits & XLHL_XMAX_EXCL_LOCK) == 0 &&
+		(infobits & XLHL_XMAX_KEYSHR_LOCK) == 0 &&
+		(infobits & XLHL_KEYS_UPDATED) == 0)
+		return;
+
+	appendStringInfoString(buf, ", infobits: [");
+
 	if (infobits & XLHL_XMAX_IS_MULTI)
-		appendStringInfoString(buf, "IS_MULTI ");
+		appendStringInfoString(buf, " IS_MULTI");
 	if (infobits & XLHL_XMAX_LOCK_ONLY)
-		appendStringInfoString(buf, "LOCK_ONLY ");
+		appendStringInfoString(buf, ", LOCK_ONLY");
 	if (infobits & XLHL_XMAX_EXCL_LOCK)
-		appendStringInfoString(buf, "EXCL_LOCK ");
+		appendStringInfoString(buf, ", EXCL_LOCK");
 	if (infobits & XLHL_XMAX_KEYSHR_LOCK)
-		appendStringInfoString(buf, "KEYSHR_LOCK ");
+		appendStringInfoString(buf, ", KEYSHR_LOCK");
 	if (infobits & XLHL_KEYS_UPDATED)
-		appendStringInfoString(buf, "KEYS_UPDATED ");
+		appendStringInfoString(buf, ", KEYS_UPDATED");
+
+	appendStringInfoString(buf, " ]");
+}
+
+static void
+plan_elem_desc(StringInfo buf, void *restrict plan, void *restrict data)
+{
+	xl_heap_freeze_plan *new_plan = (xl_heap_freeze_plan *) plan;
+	OffsetNumber **offsets = data;
+
+	appendStringInfo(buf, "{ xmax: %u, infomask: %u, infomask2: %u, ntuples: %u",
+					 new_plan->xmax, new_plan->t_infomask,
+					 new_plan->t_infomask2,
+					 new_plan->ntuples);
+
+	appendStringInfoString(buf, ", offsets:");
+	array_desc(buf, *offsets, sizeof(OffsetNumber), new_plan->ntuples,
+			   &offset_elem_desc, NULL);
+
+	*offsets += new_plan->ntuples;
+
+	appendStringInfo(buf, " }");
 }
 
+
 void
 heap_desc(StringInfo buf, XLogReaderState *record)
 {
@@ -42,14 +76,15 @@ heap_desc(StringInfo buf, XLogReaderState *record)
 	{
 		xl_heap_insert *xlrec = (xl_heap_insert *) rec;
 
-		appendStringInfo(buf, "off %u flags 0x%02X", xlrec->offnum,
+		appendStringInfo(buf, "off: %u, flags: 0x%02X",
+						 xlrec->offnum,
 						 xlrec->flags);
 	}
 	else if (info == XLOG_HEAP_DELETE)
 	{
 		xl_heap_delete *xlrec = (xl_heap_delete *) rec;
 
-		appendStringInfo(buf, "off %u flags 0x%02X ",
+		appendStringInfo(buf, "off: %u, flags: 0x%02X",
 						 xlrec->offnum,
 						 xlrec->flags);
 		out_infobits(buf, xlrec->infobits_set);
@@ -58,12 +93,12 @@ heap_desc(StringInfo buf, XLogReaderState *record)
 	{
 		xl_heap_update *xlrec = (xl_heap_update *) rec;
 
-		appendStringInfo(buf, "off %u xmax %u flags 0x%02X ",
+		appendStringInfo(buf, "off: %u, xmax: %u, flags: 0x%02X",
 						 xlrec->old_offnum,
 						 xlrec->old_xmax,
 						 xlrec->flags);
 		out_infobits(buf, xlrec->old_infobits_set);
-		appendStringInfo(buf, "; new off %u xmax %u",
+		appendStringInfo(buf, ", new off: %u, xmax %u",
 						 xlrec->new_offnum,
 						 xlrec->new_xmax);
 	}
@@ -71,39 +106,41 @@ heap_desc(StringInfo buf, XLogReaderState *record)
 	{
 		xl_heap_update *xlrec = (xl_heap_update *) rec;
 
-		appendStringInfo(buf, "off %u xmax %u flags 0x%02X ",
+		appendStringInfo(buf, "off: %u, xmax: %u, flags: 0x%02X",
 						 xlrec->old_offnum,
 						 xlrec->old_xmax,
 						 xlrec->flags);
 		out_infobits(buf, xlrec->old_infobits_set);
-		appendStringInfo(buf, "; new off %u xmax %u",
+		appendStringInfo(buf, ", new off: %u, xmax: %u",
 						 xlrec->new_offnum,
 						 xlrec->new_xmax);
 	}
 	else if (info == XLOG_HEAP_TRUNCATE)
 	{
 		xl_heap_truncate *xlrec = (xl_heap_truncate *) rec;
-		int			i;
 
+		appendStringInfoString(buf, "flags: [");
 		if (xlrec->flags & XLH_TRUNCATE_CASCADE)
-			appendStringInfoString(buf, "cascade ");
+			appendStringInfoString(buf, " CASCADE");
 		if (xlrec->flags & XLH_TRUNCATE_RESTART_SEQS)
-			appendStringInfoString(buf, "restart_seqs ");
-		appendStringInfo(buf, "nrelids %u relids", xlrec->nrelids);
-		for (i = 0; i < xlrec->nrelids; i++)
-			appendStringInfo(buf, " %u", xlrec->relids[i]);
+			appendStringInfoString(buf, ", RESTART_SEQS");
+		appendStringInfoString(buf, " ]");
+
+		appendStringInfo(buf, ", nrelids: %u", xlrec->nrelids);
+		appendStringInfoString(buf, ", relids:");
+		array_desc(buf, xlrec->relids, sizeof(Oid), xlrec->nrelids, &relid_desc, NULL);
 	}
 	else if (info == XLOG_HEAP_CONFIRM)
 	{
 		xl_heap_confirm *xlrec = (xl_heap_confirm *) rec;
 
-		appendStringInfo(buf, "off %u", xlrec->offnum);
+		appendStringInfo(buf, "off: %u", xlrec->offnum);
 	}
 	else if (info == XLOG_HEAP_LOCK)
 	{
 		xl_heap_lock *xlrec = (xl_heap_lock *) rec;
 
-		appendStringInfo(buf, "off %u: xid %u: flags 0x%02X ",
+		appendStringInfo(buf, "off: %u, xid: %u, flags: 0x%02X",
 						 xlrec->offnum, xlrec->locking_xid, xlrec->flags);
 		out_infobits(buf, xlrec->infobits_set);
 	}
@@ -111,9 +148,10 @@ heap_desc(StringInfo buf, XLogReaderState *record)
 	{
 		xl_heap_inplace *xlrec = (xl_heap_inplace *) rec;
 
-		appendStringInfo(buf, "off %u", xlrec->offnum);
+		appendStringInfo(buf, "off: %u", xlrec->offnum);
 	}
 }
+
 void
 heap2_desc(StringInfo buf, XLogReaderState *record)
 {
@@ -125,43 +163,102 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
 	{
 		xl_heap_prune *xlrec = (xl_heap_prune *) rec;
 
-		appendStringInfo(buf, "snapshotConflictHorizon %u nredirected %u ndead %u",
+		appendStringInfo(buf, "snapshotConflictHorizon: %u, nredirected: %u, ndead: %u",
 						 xlrec->snapshotConflictHorizon,
 						 xlrec->nredirected,
 						 xlrec->ndead);
+
+
+		if (!XLogRecHasBlockImage(record, 0))
+		{
+			OffsetNumber *end;
+			OffsetNumber *redirected;
+			OffsetNumber *nowdead;
+			OffsetNumber *nowunused;
+			int			nredirected;
+			int			nunused;
+			Size		datalen;
+
+			redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
+
+			nredirected = xlrec->nredirected;
+			end = (OffsetNumber *) ((char *) redirected + datalen);
+			nowdead = redirected + (nredirected * 2);
+			nowunused = nowdead + xlrec->ndead;
+			nunused = (end - nowunused);
+			Assert(nunused >= 0);
+
+			appendStringInfo(buf, ", nunused: %u", nunused);
+
+			appendStringInfoString(buf, ", redirected:");
+			array_desc(buf, redirected, sizeof(OffsetNumber) * 2, nredirected * 2, &redirect_elem_desc, NULL);
+			appendStringInfoString(buf, ", dead:");
+			array_desc(buf, nowdead, sizeof(OffsetNumber), xlrec->ndead, &offset_elem_desc, NULL);
+			appendStringInfoString(buf, ", unused:");
+			array_desc(buf, nowunused, sizeof(OffsetNumber), nunused, &offset_elem_desc, NULL);
+		}
 	}
 	else if (info == XLOG_HEAP2_VACUUM)
 	{
 		xl_heap_vacuum *xlrec = (xl_heap_vacuum *) rec;
 
-		appendStringInfo(buf, "nunused %u", xlrec->nunused);
+		appendStringInfo(buf, "nunused: %u", xlrec->nunused);
+
+		if (!XLogRecHasBlockImage(record, 0))
+		{
+			OffsetNumber *nowunused;
+
+			nowunused = (OffsetNumber *) XLogRecGetBlockData(record, 0, NULL);
+
+			appendStringInfoString(buf, ", unused:");
+			array_desc(buf, nowunused, sizeof(OffsetNumber), xlrec->nunused, &offset_elem_desc, NULL);
+		}
 	}
 	else if (info == XLOG_HEAP2_FREEZE_PAGE)
 	{
 		xl_heap_freeze_page *xlrec = (xl_heap_freeze_page *) rec;
 
-		appendStringInfo(buf, "snapshotConflictHorizon %u nplans %u",
+		appendStringInfo(buf, "snapshotConflictHorizon: %u, nplans: %u",
 						 xlrec->snapshotConflictHorizon, xlrec->nplans);
+
+		if (!XLogRecHasBlockImage(record, 0))
+		{
+			xl_heap_freeze_plan *plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, NULL);
+			OffsetNumber *offsets = (OffsetNumber *) &plans[xlrec->nplans];
+
+			appendStringInfoString(buf, ", plans:");
+			array_desc(buf,
+					   plans,
+					   sizeof(xl_heap_freeze_plan),
+					   xlrec->nplans,
+					   &plan_elem_desc,
+					   &offsets);
+
+		}
 	}
 	else if (info == XLOG_HEAP2_VISIBLE)
 	{
 		xl_heap_visible *xlrec = (xl_heap_visible *) rec;
 
-		appendStringInfo(buf, "snapshotConflictHorizon %u flags 0x%02X",
+		appendStringInfo(buf, "snapshotConflictHorizon: %u, flags: 0x%02X",
 						 xlrec->snapshotConflictHorizon, xlrec->flags);
 	}
 	else if (info == XLOG_HEAP2_MULTI_INSERT)
 	{
 		xl_heap_multi_insert *xlrec = (xl_heap_multi_insert *) rec;
+		bool		isinit = (XLogRecGetInfo(record) & XLOG_HEAP_INIT_PAGE) != 0;
 
-		appendStringInfo(buf, "%d tuples flags 0x%02X", xlrec->ntuples,
+		appendStringInfo(buf, "ntuples: %d, flags: 0x%02X", xlrec->ntuples,
 						 xlrec->flags);
+		appendStringInfoString(buf, ", offsets:");
+		if (!XLogRecHasBlockImage(record, 0) && !isinit)
+			array_desc(buf, xlrec->offsets, sizeof(OffsetNumber), xlrec->ntuples, &offset_elem_desc, NULL);
 	}
 	else if (info == XLOG_HEAP2_LOCK_UPDATED)
 	{
 		xl_heap_lock_updated *xlrec = (xl_heap_lock_updated *) rec;
 
-		appendStringInfo(buf, "off %u: xmax %u: flags 0x%02X ",
+		appendStringInfo(buf, "off: %u, xmax: %u, flags: 0x%02X",
 						 xlrec->offnum, xlrec->xmax, xlrec->flags);
 		out_infobits(buf, xlrec->infobits_set);
 	}
@@ -169,13 +266,13 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
 	{
 		xl_heap_new_cid *xlrec = (xl_heap_new_cid *) rec;
 
-		appendStringInfo(buf, "rel %u/%u/%u; tid %u/%u",
+		appendStringInfo(buf, "rel: %u/%u/%u, tid: %u/%u",
 						 xlrec->target_locator.spcOid,
 						 xlrec->target_locator.dbOid,
 						 xlrec->target_locator.relNumber,
 						 ItemPointerGetBlockNumber(&(xlrec->target_tid)),
 						 ItemPointerGetOffsetNumber(&(xlrec->target_tid)));
-		appendStringInfo(buf, "; cmin: %u, cmax: %u, combo: %u",
+		appendStringInfo(buf, ", cmin: %u, cmax: %u, combo: %u",
 						 xlrec->cmin, xlrec->cmax, xlrec->combocid);
 	}
 }
diff --git a/src/backend/access/rmgrdesc/meson.build b/src/backend/access/rmgrdesc/meson.build
index 166cee67b6..f76e87e2d7 100644
--- a/src/backend/access/rmgrdesc/meson.build
+++ b/src/backend/access/rmgrdesc/meson.build
@@ -16,6 +16,7 @@ rmgr_desc_sources = files(
   'nbtdesc.c',
   'relmapdesc.c',
   'replorigindesc.c',
+  'rmgrdesc_utils.c',
   'seqdesc.c',
   'smgrdesc.c',
   'spgdesc.c',
diff --git a/src/backend/access/rmgrdesc/rmgrdesc_utils.c b/src/backend/access/rmgrdesc/rmgrdesc_utils.c
new file mode 100644
index 0000000000..fa45c796a0
--- /dev/null
+++ b/src/backend/access/rmgrdesc/rmgrdesc_utils.c
@@ -0,0 +1,88 @@
+/*-------------------------------------------------------------------------
+ *
+ * rmgrdesc_utils.c
+ *		support functions for rmgrdesc
+ *
+ * Portions Copyright (c) 2023, PostgreSQL Global Development Group
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/access/rmgrdesc/rmgrdesc_utils.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "access/rmgrdesc_utils.h"
+
+/*
+ * The expected formatting of a desc function for an xlog record is as follows:
+ * member1_name: member1_value, member2_name: member2_value
+ * If the value is a list, please use
+ * member3_name: [ member3_list_value1, member3_list_value2 ]
+ *
+ * The first item appended to the string should not be prepended by any spaces
+ * or comma, however all subsequent appends to the string are responsible for
+ * prepended themselves with a comma followed by a space.
+ *
+ * Arrays should have a space between the opening square bracket and first
+ * element and between the last element and closing brace.
+ *
+ * Flags should be in all caps.
+ *
+ * For lists/arrays of items, the number of those items should be listed at the
+ * beginning with all the other numbers.
+ *
+ * whether or not to leave out a n[item] if the number is 0 (not consistent for
+ * XLOG_HEAP2_VACUUM and XLOG_HEAP2_PRUNE
+ *
+ * composite objects in a list should be surrounded with { }
+ *
+ * etc.
+ *
+ * TODO: make this description better/maybe put in a README
+ */
+
+
+void
+array_desc(StringInfo buf, void *array, size_t elem_size, int count,
+		   void (*elem_desc) (StringInfo buf, void *restrict elem, void *restrict data),
+		   void *restrict data)
+{
+	if (count == 0)
+	{
+		appendStringInfoString(buf, " []");
+		return;
+	}
+	appendStringInfo(buf, " [");
+	for (int i = 0; i < count; i++)
+	{
+		if (i > 0)
+			appendStringInfoString(buf, ",");
+		appendStringInfoString(buf, " ");
+
+		elem_desc(buf, (char *) array + elem_size * i, data);
+	}
+	appendStringInfoString(buf, " ]");
+}
+
+void
+redirect_elem_desc(StringInfo buf, void *restrict offset, void *restrict data)
+{
+	OffsetNumber *new_offset = (OffsetNumber *) offset;
+
+	appendStringInfo(buf, "%u->%u", new_offset[0], new_offset[1]);
+}
+
+void
+offset_elem_desc(StringInfo buf, void *restrict offset, void *restrict data)
+{
+	appendStringInfo(buf, "%u", *(OffsetNumber *) offset);
+}
+
+void
+relid_desc(StringInfo buf, void *restrict relid, void *restrict data)
+{
+	appendStringInfo(buf, "%u", *(Oid *) relid);
+}
diff --git a/src/bin/pg_waldump/Makefile b/src/bin/pg_waldump/Makefile
index d6459e17c7..61c9de470c 100644
--- a/src/bin/pg_waldump/Makefile
+++ b/src/bin/pg_waldump/Makefile
@@ -18,7 +18,7 @@ OBJS = \
 
 override CPPFLAGS := -DFRONTEND $(CPPFLAGS)
 
-RMGRDESCSOURCES = $(sort $(notdir $(wildcard $(top_srcdir)/src/backend/access/rmgrdesc/*desc.c)))
+RMGRDESCSOURCES = $(sort $(notdir $(wildcard $(top_srcdir)/src/backend/access/rmgrdesc/*desc.c) $(top_srcdir)/src/backend/access/rmgrdesc/rmgrdesc_utils.c))
 RMGRDESCOBJS = $(patsubst %.c,%.o,$(RMGRDESCSOURCES))
 
 
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index a2c67d1cd3..63075c51bd 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -265,6 +265,7 @@ typedef struct xl_heap_vacuum
 #define SizeOfHeapVacuum (offsetof(xl_heap_vacuum, nunused) + sizeof(uint16))
 
 /* flags for infobits_set */
+// add a comment about this
 #define XLHL_XMAX_IS_MULTI		0x01
 #define XLHL_XMAX_LOCK_ONLY		0x02
 #define XLHL_XMAX_EXCL_LOCK		0x04
diff --git a/src/include/access/rmgrdesc_utils.h b/src/include/access/rmgrdesc_utils.h
new file mode 100644
index 0000000000..7e4fcbdf25
--- /dev/null
+++ b/src/include/access/rmgrdesc_utils.h
@@ -0,0 +1,33 @@
+/*-------------------------------------------------------------------------
+ *
+ * rmgrdesc_utils.h
+ *	  helper utilities for rmgrdesc
+ *
+ * Copyright (c) 2023 PostgreSQL Global Development Group
+ *
+ * src/include/access/rmgrdesc_utils.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef RMGRDESC_UTILS_H_
+#define RMGRDESC_UTILS_H_
+
+#include "storage/off.h"
+#include "access/heapam_xlog.h"
+
+void
+			array_desc(StringInfo buf, void *array, size_t elem_size, int count,
+					   void (*elem_desc) (StringInfo buf, void *restrict elem, void *restrict data),
+					   void *restrict data);
+
+void
+			offset_elem_desc(StringInfo buf, void *restrict offset, void *restrict data);
+
+void
+			redirect_elem_desc(StringInfo buf, void *restrict offset, void *restrict data);
+
+void
+			relid_desc(StringInfo buf, void *restrict relid, void *restrict data);
+
+
+#endif							/* RMGRDESC_UTILS_H */
-- 
2.37.2

From 9a8a9d01bcd2e18e5f6240c7aedd0f982734c860 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Mon, 13 Mar 2023 18:15:17 -0400
Subject: [PATCH v2 2/2] Add detail to some btree xlog records

---
 src/backend/access/rmgrdesc/nbtdesc.c        | 81 +++++++++++++++++---
 src/backend/access/rmgrdesc/rmgrdesc_utils.c |  6 ++
 src/include/access/rmgrdesc_utils.h          |  3 +
 3 files changed, 78 insertions(+), 12 deletions(-)

diff --git a/src/backend/access/rmgrdesc/nbtdesc.c b/src/backend/access/rmgrdesc/nbtdesc.c
index c5dc543a0f..3f2f19aad7 100644
--- a/src/backend/access/rmgrdesc/nbtdesc.c
+++ b/src/backend/access/rmgrdesc/nbtdesc.c
@@ -15,6 +15,53 @@
 #include "postgres.h"
 
 #include "access/nbtxlog.h"
+#include "access/rmgrdesc_utils.h"
+
+static void
+btree_update_elem_desc(StringInfo buf, void *restrict update, void *restrict data)
+{
+	xl_btree_update *new_update = (xl_btree_update *) update;
+	OffsetNumber *updated_offset = *((OffsetNumber **) data);
+
+	appendStringInfo(buf, "{ updated offset: %u, ndeleted tids: %u", *updated_offset, new_update->ndeletedtids);
+
+	appendStringInfoString(buf, ", deleted tids:");
+
+	array_desc(buf, (char *) new_update + SizeOfBtreeUpdate,
+			sizeof(uint16), new_update->ndeletedtids, &uint16_elem_desc, NULL);
+
+	updated_offset++;
+
+	appendStringInfo(buf, " }");
+}
+
+static void
+btree_del_desc(StringInfo buf, char *block_data, uint16 ndeleted, uint16 nupdated)
+{
+	OffsetNumber *updatedoffsets;
+	xl_btree_update *updates;
+	OffsetNumber *data = (OffsetNumber *) block_data;
+
+	appendStringInfoString(buf, ", deleted:");
+	array_desc(buf, data, sizeof(OffsetNumber), ndeleted, &offset_elem_desc, NULL);
+
+	appendStringInfoString(buf, ", updated:");
+	array_desc(buf, data, sizeof(OffsetNumber), nupdated, &offset_elem_desc, NULL);
+
+	if (nupdated <= 0)
+		return;
+
+	updatedoffsets = (OffsetNumber *)
+		((char *) data + ndeleted * sizeof(OffsetNumber));
+	updates = (xl_btree_update *) ((char *) updatedoffsets +
+								nupdated *
+								sizeof(OffsetNumber));
+
+	appendStringInfoString(buf, ", updates:");
+	array_desc(buf, updates, sizeof(xl_btree_update),
+			nupdated, &btree_update_elem_desc,
+			&updatedoffsets);
+}
 
 void
 btree_desc(StringInfo buf, XLogReaderState *record)
@@ -31,7 +78,7 @@ btree_desc(StringInfo buf, XLogReaderState *record)
 			{
 				xl_btree_insert *xlrec = (xl_btree_insert *) rec;
 
-				appendStringInfo(buf, "off %u", xlrec->offnum);
+				appendStringInfo(buf, "off: %u", xlrec->offnum);
 				break;
 			}
 		case XLOG_BTREE_SPLIT_L:
@@ -39,7 +86,7 @@ btree_desc(StringInfo buf, XLogReaderState *record)
 			{
 				xl_btree_split *xlrec = (xl_btree_split *) rec;
 
-				appendStringInfo(buf, "level %u, firstrightoff %d, newitemoff %d, postingoff %d",
+				appendStringInfo(buf, "level: %u, firstrightoff: %d, newitemoff: %d, postingoff: %d",
 								 xlrec->level, xlrec->firstrightoff,
 								 xlrec->newitemoff, xlrec->postingoff);
 				break;
@@ -48,31 +95,41 @@ btree_desc(StringInfo buf, XLogReaderState *record)
 			{
 				xl_btree_dedup *xlrec = (xl_btree_dedup *) rec;
 
-				appendStringInfo(buf, "nintervals %u", xlrec->nintervals);
+				appendStringInfo(buf, "nintervals: %u", xlrec->nintervals);
 				break;
 			}
 		case XLOG_BTREE_VACUUM:
 			{
 				xl_btree_vacuum *xlrec = (xl_btree_vacuum *) rec;
 
-				appendStringInfo(buf, "ndeleted %u; nupdated %u",
-								 xlrec->ndeleted, xlrec->nupdated);
+				appendStringInfo(buf, "ndeleted: %u, nupdated: %u",
+						xlrec->ndeleted, xlrec->nupdated);
+
+				if (!XLogRecHasBlockImage(record, 0))
+					btree_del_desc(buf, XLogRecGetBlockData(record, 0, NULL),
+							xlrec->ndeleted, xlrec->nupdated);
+
 				break;
 			}
 		case XLOG_BTREE_DELETE:
 			{
 				xl_btree_delete *xlrec = (xl_btree_delete *) rec;
 
-				appendStringInfo(buf, "snapshotConflictHorizon %u; ndeleted %u; nupdated %u",
+				appendStringInfo(buf, "snapshotConflictHorizon: %u, ndeleted: %u, nupdated: %u",
 								 xlrec->snapshotConflictHorizon,
 								 xlrec->ndeleted, xlrec->nupdated);
+
+				if (!XLogRecHasBlockImage(record, 0))
+					btree_del_desc(buf, XLogRecGetBlockData(record, 0, NULL),
+							xlrec->ndeleted, xlrec->nupdated);
+
 				break;
 			}
 		case XLOG_BTREE_MARK_PAGE_HALFDEAD:
 			{
 				xl_btree_mark_page_halfdead *xlrec = (xl_btree_mark_page_halfdead *) rec;
 
-				appendStringInfo(buf, "topparent %u; leaf %u; left %u; right %u",
+				appendStringInfo(buf, "topparent: %u; leaf: %u; left: %u; right: %u",
 								 xlrec->topparent, xlrec->leafblk, xlrec->leftblk, xlrec->rightblk);
 				break;
 			}
@@ -81,11 +138,11 @@ btree_desc(StringInfo buf, XLogReaderState *record)
 			{
 				xl_btree_unlink_page *xlrec = (xl_btree_unlink_page *) rec;
 
-				appendStringInfo(buf, "left %u; right %u; level %u; safexid %u:%u; ",
+				appendStringInfo(buf, "left: %u; right: %u; level: %u; safexid: %u:%u; ",
 								 xlrec->leftsib, xlrec->rightsib, xlrec->level,
 								 EpochFromFullTransactionId(xlrec->safexid),
 								 XidFromFullTransactionId(xlrec->safexid));
-				appendStringInfo(buf, "leafleft %u; leafright %u; leaftopparent %u",
+				appendStringInfo(buf, "leafleft: %u; leafright: %u; leaftopparent: %u",
 								 xlrec->leafleftsib, xlrec->leafrightsib,
 								 xlrec->leaftopparent);
 				break;
@@ -94,14 +151,14 @@ btree_desc(StringInfo buf, XLogReaderState *record)
 			{
 				xl_btree_newroot *xlrec = (xl_btree_newroot *) rec;
 
-				appendStringInfo(buf, "lev %u", xlrec->level);
+				appendStringInfo(buf, "lev: %u", xlrec->level);
 				break;
 			}
 		case XLOG_BTREE_REUSE_PAGE:
 			{
 				xl_btree_reuse_page *xlrec = (xl_btree_reuse_page *) rec;
 
-				appendStringInfo(buf, "rel %u/%u/%u; snapshotConflictHorizon %u:%u",
+				appendStringInfo(buf, "rel: %u/%u/%u, snapshotConflictHorizon: %u:%u",
 								 xlrec->locator.spcOid, xlrec->locator.dbOid,
 								 xlrec->locator.relNumber,
 								 EpochFromFullTransactionId(xlrec->snapshotConflictHorizon),
@@ -114,7 +171,7 @@ btree_desc(StringInfo buf, XLogReaderState *record)
 
 				xlrec = (xl_btree_metadata *) XLogRecGetBlockData(record, 0,
 																  NULL);
-				appendStringInfo(buf, "last_cleanup_num_delpages %u",
+				appendStringInfo(buf, "last_cleanup_num_delpages: %u",
 								 xlrec->last_cleanup_num_delpages);
 				break;
 			}
diff --git a/src/backend/access/rmgrdesc/rmgrdesc_utils.c b/src/backend/access/rmgrdesc/rmgrdesc_utils.c
index fa45c796a0..26da7e344f 100644
--- a/src/backend/access/rmgrdesc/rmgrdesc_utils.c
+++ b/src/backend/access/rmgrdesc/rmgrdesc_utils.c
@@ -86,3 +86,9 @@ relid_desc(StringInfo buf, void *restrict relid, void *restrict data)
 {
 	appendStringInfo(buf, "%u", *(Oid *) relid);
 }
+
+void
+uint16_elem_desc(StringInfo buf, void *restrict value, void *restrict data)
+{
+	appendStringInfo(buf, "%u", *(uint16 *) value);
+}
diff --git a/src/include/access/rmgrdesc_utils.h b/src/include/access/rmgrdesc_utils.h
index 7e4fcbdf25..ddf50335b1 100644
--- a/src/include/access/rmgrdesc_utils.h
+++ b/src/include/access/rmgrdesc_utils.h
@@ -29,5 +29,8 @@ void
 void
 			relid_desc(StringInfo buf, void *restrict relid, void *restrict data);
 
+void
+			uint16_elem_desc(StringInfo buf, void *restrict value, void *restrict data);
+
 
 #endif							/* RMGRDESC_UTILS_H */
-- 
2.37.2

Reply via email to