On Fri, Apr 7, 2023 at 1:33 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> Attached v3 is cleaned up and includes a pg_walinspect docs update as
> well as some edited comments in rmgr_utils.c
Attached v4 has some small tweaks on your v3. Mostly just whitespace
tweaks. Two slightly notable tweaks:
* I changed the approach to globbing in the Makefile, rather than use
your original overwide formulation for the new rmgrdesc_utils.c file.
What do you think of this approach?
* Removed use of the restrict keyword.
While "restrict" is C99, I'm not completely sure that it's totally
supported by Postgres. I'm a bit surprised that you opted to use it in
this particular patch.
I meant to ask you about this earlier...why use restrict in this patch?
> I've added such an example to pg_walinspect docs.
There already was a PRUNE example, though -- for the
pg_get_wal_record_info function (singular, not to be confused with
pg_get_wal_records_info).
v4 makes the example a VACUUM record, which replaces the previous
pg_get_wal_record_info PRUNE example -- that needed to be updated
anyway. This approach has the advantage of not being too verbose,
which still showing some of this kind of detail.
This has the advantage of allowing pg_get_wal_records_info's example
to continue to be an example that lacks a block reference (and so has
a NULL block_ref). This is a useful contrast against the new
pg_get_wal_block_info function.
> I really like this idea and would find it useful. I reviewed the patch
> and tried it out and it worked for me and code looked fine as well.
>
> I didn't include it in the attached patchset because I don't feel
> confident enough in my own understanding of any potential implications
> of splitting up these record types to definitively endorse it. But, if
> someone else felt comfortable with it, I would like to see it in the
> tree.
I'm not going to move on it now for 16, given the lack of feedback about it.
--
Peter Geoghegan
