Re: logical decoding/replication: new functions pg_ls_logicaldir and pg_ls_replslotdir

On Sun, Nov 21, 2021 at 6:58 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Sat, Nov 20, 2021 at 12:29:51AM +0000, Bossart, Nathan wrote:
> > On 11/17/21, 11:39 PM, "Bharath Rupireddy" <bharath.rupireddyforpostgres@gmail.com> wrote:
> >> Please review the attached v2.
> >
> > LGTM.  I've marked this one as ready-for-committer.
>
> One issue that I have with this patch is that there are zero
> regression tests.  Could you add a couple of things in
> misc_functions.sql (for the negative tests perhaps) or
> contrib/test_decoding/, taking advantage of places where slots are
> already created?  You may want to look after the non-superuser case
> where the calls should fail, and the second case where a role is part
> of pg_monitor where the call succeeds.  Note that any roles created in
> the tests have to be prefixed with "regress_".

I don't think we need to go far to contrib/test_decoding/, even if we
add it there we can't test it for the outputs of these functions, so
I've added the tests in misc_functinos.sql itself.

> +   snprintf(path, sizeof(path), "%s/%s", "pg_replslot", slotname);
> +   return pg_ls_dir_files(fcinfo, path, false);
> "pg_replslot" could be part of the third argument here.  There is no
> need to separate it.

Done.

> +        ordinary file in the server's pg_logical/mappings directory.
> Paths had better have <filename> markups around them, no?

Done.

Attached v3 patch, please review it further.

Regards,
Bharath Rupireddy.