Re: Add contrib/pg_logicalsnapinspect

On Tue, Sep 24, 2024 at 10:23 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Sep 24, 2024 at 09:15:31AM +0530, shveta malik wrote:
> > On Fri, Sep 20, 2024 at 12:22 PM Bertrand Drouvot
> > <bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> > >
> > >
> > > Please find attached v8, that:
> > >
> >
> > Thank You for the patch. In one of my tests, I noticed that I got
> > negative checksum:
> >
> > postgres=# SELECT * FROM pg_get_logical_snapshot_meta('0/3481F20');
> > magic | checksum | version
> > ------------+------------+---------
> > 1369563137 | -266346460 | 6
> >
> > But pg_crc32c is uint32. Is it because we are getting it as
> > Int32GetDatum(ondisk.checksum) in pg_get_logical_snapshot_meta()?
> > Instead should it be UInt32GetDatum?
>
> Thanks for the testing.
>
> As the checksum could be > 2^31 - 1, then v9 (just shared up-thread) changes it
> to an int8 in the pg_logicalinspect--1.0.sql file. So, to avoid CI failure on
> the 32bit build, then v9 is using Int64GetDatum() instead of UInt32GetDatum().
>

Okay, looks good,

> > Same goes for below:
> > values[i++] = Int32GetDatum(ondisk.magic);
> > values[i++] = Int32GetDatum(ondisk.magic);
>
> The 2 others field (magic and version) are unlikely to be > 2^31 - 1, so v9 is
> making use of UInt32GetDatum() and keep int4 in the sql file.
>
> > We need to recheck the rest of the fields in the info() function as well.
>
> I think that the pg_get_logical_snapshot_info()'s fields are ok (I did spend some
> time to debug CI failing on the 32bit build for some on them before submitting v1).
>

+ OUT catchange_xip xid[]

One question, what is xid datatype, is it too int8? Sorry, could not
find the correct doc. Since we are getting uint32 in Int64, this also
needs to be accordingly.

thanks
Shveta