From: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
---|---|
To: | Peter Geoghegan <pg(at)bowt(dot)ie> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
Subject: | Re: Show various offset arrays for heap WAL records |
Date: | 2023-04-07 23:01:29 |
Message-ID: | CAAKRu_brhCVyCbUxxK548GBFA7YJ=H5mEJ9Lj97iRO-WM7aB+Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Apr 7, 2023 at 5:43 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> On Fri, Apr 7, 2023 at 1:33 PM Melanie Plageman
> <melanieplageman(at)gmail(dot)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?
Seems fine.
> * 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?
So, I think the signature I meant to have was:
void
array_desc(StringInfo buf, void *array, size_t elem_size, int count,
void (*elem_desc) (StringInfo buf, const void *elem, void *data),
void *data)
Basically I wanted to indicate that elem was not and should not be
modified and data can be modified but that they should not be the same
element or overlap at all.
> > 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.
LGTM
- Melanie
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2023-04-07 23:09:16 | Re: Show various offset arrays for heap WAL records |
Previous Message | Daniel Gustafsson | 2023-04-07 22:59:18 | Re: Making background psql nicer to use in tap tests |