Re: Add contrib/pg_logicalsnapinspect

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().

> 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).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com