Re: Add contrib/pg_logicalsnapinspect

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