Re: Add contrib/pg_logicalsnapinspect

On Tue, Sep 17, 2024 at 10:46 AM David G. Johnston
<david(dot)g(dot)johnston(at)gmail(dot)com> 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.
>
> 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.

Okay, understood. Thanks for explaining.

> 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.

Okay, a single row can be investigated if it comes under this scope.
But I see why 'ERROR' is not a possibility here.

thanks
Shveta