Re: Add contrib/pg_logicalsnapinspect

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.

If there is exactly one single input argument function of the given name
the parser is going to cast the null literal to the data type of the single
argument and invoke the function. It will not and cannot be convinced to
fail to find a matching function.

I can see an argument that they should produce an empty set instead of a
single all-null row, but the idea that they wouldn’t even be found is
contrary to a core design of the system.

David J.