On Thu, 5 Jan 2023 at 18:52, Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Wed, Jan 4, 2023 at 8:19 PM Drouvot, Bertrand
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > I think it makes sense to somehow align the pg_walinspect functions with the pg_waldump "features".
> > And since [1] added FPI "extraction" then +1 for the proposed patch in this thread.
> >
> > > [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=d497093cbecccf6df26365e06a5f8f8614b591c8
> > > [2] https://postgr.es/m/CAOxo6XKjQb2bMSBRpePf3ZpzfNTwjQUc4Tafh21=jzjX6bX8CA@mail.gmail.com
> >
> > I just have a few comments:
>
> Thanks for reviewing.
>
> > +
> > +/*
> > + * Get full page images and their info associated with a given WAL record.
> > + */
> >
> >
> > + <para>
> > + Gets raw full page images and their information associated with all the
> > + valid WAL records between <replaceable>start_lsn</replaceable> and
> > + <replaceable>end_lsn</replaceable>. Returns one row per full page image.
> >
> > Worth to add a few words about decompression too?
>
> Done.
>
> > What about adding a few words about compression? (like "Decompression is applied if necessary"?)
> >
> >
> > + /* Full page exists, so let's output it. */
> > + if (!RestoreBlockImage(record, block_id, page))
> >
> > "Full page exists, so let's output its info and content." instead?
>
> Done.
>
> > I'm also wondering if it would make sense to extend the test coverage of it (and pg_waldump) to "validate" that
both
> > extracted images are the same and matches the one modified right after the checkpoint.
> >
> > What do you think? (could be done later in another patch though).
>
> I think pageinspect can be used here. We can fetch the raw page from
> the table after the checkpoint and raw FPI from the WAL record logged
> as part of the update. I've tried to do so [1], but I see a slight
> difference in the raw output. The expectation is that they both be the
> same. It might be that the update operation logs the FPI with some
> more info set (prune_xid). I'll try to see why it is so.
>
> I'm attaching the v2 patch for further review.
I felt one of the files was missing in the patch:
[13:39:03.534] contrib/pg_walinspect/meson.build:19:0: ERROR: File
pg_walinspect--1.0--1.1.sql does not exist.
Please post an updated version for the same.
Regards,
Vignesh