Re: Add contrib/pg_logicalsnapinspect

Hi,

On Mon, Sep 16, 2024 at 10:16:16PM -0700, David G. Johnston wrote:
> On Monday, September 16, 2024, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> > On Tue, Sep 17, 2024 at 10:18 AM shveta malik <shveta(dot)malik(at)gmail(dot)com>
> > wrote:
> > >
> > > Thanks for addressing the comments. I have not started reviewing v4
> > > yet, but here are few more comments on v3:
> > >
> >
> > I just noticed that when we pass NULL input, both the new functions
> > give 1 row as output, all cols as NULL:
> >
> > newdb1=# SELECT * FROM pg_get_logical_snapshot_meta(NULL);
> > magic | checksum | version
> > -------+----------+---------
> > | |
> >
> > (1 row)
> >
> > Similar behavior with pg_get_logical_snapshot_info(). While the
> > existing 'pg_ls_logicalsnapdir' function gives this error, which looks
> > more meaningful:
> >
> > newdb1=# select * from pg_ls_logicalsnapdir(NULL);
> > ERROR: function pg_ls_logicalsnapdir(unknown) does not exist
> > LINE 1: select * from pg_ls_logicalsnapdir(NULL);
> > HINT: No function matches the given name and argument types. You
> > might need to add explicit type casts.
> >
> >
> > Shouldn't the new functions have same behavior?
> >
>
> No. Since the name pg_ls_logicalsnapdir has zero single-argument
> implementations passing a null value as an argument is indeed attempt to
> invoke a function signature that doesn’t exist.

Agree.

> I can see an argument that they should produce an empty set instead of a
> single all-null row,

Yeah, it's outside the scope of this patch but I've seen different behavior
in this area.

For example:

postgres=# select * from pg_ls_replslotdir(NULL);
name | size | modification
------+------+--------------
(0 rows)

as compared to:

postgres=# select * from pg_walfile_name_offset(NULL);
file_name | file_offset
-----------+-------------
|
(1 row)

I thought that it might be linked to the volatility but it is not:

postgres=# select * from pg_stat_get_xact_blocks_fetched(NULL);
pg_stat_get_xact_blocks_fetched
---------------------------------

(1 row)

postgres=# select * from pg_get_multixact_members(NULL);
xid | mode
-----+------
(0 rows)

while both are volatile.

I think both make sense: It's "empty" or we "don't know the values of the fields".
I don't know if there is any reason about this "inconsistency".

Regards,

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