On Thu, 5 Jan 2023 at 18:52, Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Wed, Jan 4, 2023 at 8:19 PM Drouvot, Bertrand > <bertranddrouvot...@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=jzjx6bx...@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
