Re: psql: \dl+ to list large objects privileges

Hi,

On 03.09.2021 15:25, Georgios Kokolatos wrote:
> On a high level I will recommend the addition of tests. There are similar tests

Tests added.

> Applying the patch, generates several whitespace warnings. It will be helpful
> if those warnings are removed.

I know this is a silly mistake, and after reading this article[1] I
tried to remove the extra spaces.

Can you tell me, please, how can you get such warnings?

> The patch contains:
>
> case 'l':
> - success = do_lo_list();
> + success = listLargeObjects(show_verbose);
>
>
> It might be of some interest to consider in the above to check the value of the
> next character in command or emit an error if not valid. Such a pattern can be
> found in the same switch block as for example:
>
> switch (cmd[2])
> {
> case '\0':
> case '+':
> <snip>
> success = ...
> </snip>
> break;
> default:
> status = PSQL_CMD_UNKNOWN;
> break;
> }

Check added.

> The patch contains:
>
> else if (strcmp(cmd + 3, "list") == 0)
> - success = do_lo_list();
> + success = listLargeObjects(false);
> +
> + else if (strcmp(cmd + 3, "list+") == 0)
> + success = listLargeObjects(true);
>
>
> In a fashion similar to `exec_command_list`, it might be interesting to consider
> expressing the above as:
>
> show_verbose = strchr(cmd, '+') ? true : false;
> <snip>
> else if (strcmp(cmd + 3, "list") == 0
> success = do_lo_list(show_verbose);

I rewrote this part.

New version attached.

[1] https://wiki.postgresql.org/wiki/Creating_Clean_Patches

--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
The Russian Postgres Company