Horton

Re: Add contrib/pg_logicalsnapinspect

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

View thread
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Peter Smith <smithpb2250@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-09-20T06:52:17Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix indentation issue

  2. pg_logicalinspect: Stabilize isolation tests.

  3. Add contrib/pg_logicalinspect.

  4. Move SnapBuild and SnapBuildOnDisk structs to snapshot_internal.h.

Attachments

Hi,

On Thu, Sep 19, 2024 at 10:08:19AM +1000, Peter Smith wrote:
> Thanks for the updated patch.
> 
> ======
> .../expected/logical_inspect.out
> 
> 2
> +step s1_get_logical_snapshot_info: SELECT
> (pg_get_logical_snapshot_info(f.name::pg_lsn)).state,(pg_get_logical_snapshot_info(f.name::pg_lsn)).catchange_count,array_length((pg_get_logical_snapshot_info(f.name::pg_lsn)).catchange_xip,1),(pg_get_logical_snapshot_info(f.name::pg_lsn)).committed_count,array_length((pg_get_logical_snapshot_info(f.name::pg_lsn)).committed_xip,1)
> FROM (SELECT replace(replace(name,'.snap',''),'-','/') AS name FROM
> pg_ls_logicalsnapdir()) AS f ORDER BY 2;
> +state|catchange_count|array_length|committed_count|array_length
> +-----+---------------+------------+---------------+------------
> +    2|              0|            |              2|           2
> +    2|              2|           2|              0|
> +(2 rows)
> +
> 
> 2a.
> Would it be better to rearrange those columns so 'committed' stuff
> comes before 'catchange' stuff, to make this table order consistent
> with the structure/code?

I'm not sure that's a good idea to create a "dependency" between the test output
and the code. I think that could be hard to "ensure" in the mid-long term.

Please find attached v8, that:

- takes care of your comments (except the one above)
- takes care of Shveta comment [1]
- displays the 'text' for the state column (as confirmed by Amit [2]): Note that
the enum -> text mapping is done in pg_logicalinspect.c and a comment has been
added in snapbuild.h (near the SnapBuildState definition). I thought it makes
more sense to do it that way instead of implementing the enum -> text mapping
in snapbuild.h (as the mapping is only used by the module).

[1]: https://www.postgresql.org/message-id/CAJpy0uDJ65QHUZfww7n6TBZAGp-SP74P5U3fUorV%2B%3DbaaRu6Dw%40mail.gmail.com
[2]: https://www.postgresql.org/message-id/CAA4eK1JgW1o9wOTwgRJ9%2BbQkYcr3iRWAQHoL9eBC%2BrmoQoHZ%3DQ%40mail.gmail.com

Regards,

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