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

From: Georgios Kokolatos <gkokolatos(at)protonmail(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Pavel Luzanov <pluzanov(at)postgrespro(dot)ru>
Subject: Re: psql: \dl+ to list large objects privileges
Date: 2021-09-03 12:25:44
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested


thank you for the patch, I personally think it is a useful addition and thus it
gets my vote. However, I also think that the proposed code will need some

On a high level I will recommend the addition of tests. There are similar tests
present in:

I will also inquire as to the need for renaming the function `do_lo_list` to
`listLargeObjects` and its move to describe.c. from large_obj.c. In itself it is
not necessarily a blocking point, though it will require some strong arguments
for doing so.

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

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 '+':
success = ...

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;
else if (strcmp(cmd + 3, "list") == 0
success = do_lo_list(show_verbose);



In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Georgios Kokolatos 2021-09-03 12:45:14 Re: psql: \dl+ to list large objects privileges
Previous Message Amit Kapila 2021-09-03 11:18:58 Re: Added schema level support for publication.