Hi, On Mon, Sep 16, 2024 at 10:16:16PM -0700, David G. Johnston wrote: > On Monday, September 16, 2024, shveta malik <shveta.ma...@gmail.com> wrote: > > > On Tue, Sep 17, 2024 at 10:18 AM shveta malik <shveta.ma...@gmail.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