Re: psql: \dl+ to list large objects privileges
От
Georgios Kokolatos
Тема
Re: psql: \dl+ to list large objects privileges
Дата
Msg-id
163067194414.1167.8140856874411625212.pgcf@coridan.postgresql.org
Ответ на
Re: psql: \dl+ to list large objects privileges (Pavel Luzanov)
Список
Дерево обсуждения
psql: \dl+ to list large objects privileges Pavel Luzanov <p.luzanov@postgrespro.ru>
Re: psql: \dl+ to list large objects privileges Justin Pryzby <pryzby@telsasoft.com>
Re: psql: \dl+ to list large objects privileges Tom Lane <tgl@sss.pgh.pa.us>
Re: psql: \dl+ to list large objects privileges Pavel Luzanov <p.luzanov@postgrespro.ru>
Re: psql: \dl+ to list large objects privileges Tom Lane <tgl@sss.pgh.pa.us>
Re: psql: \dl+ to list large objects privileges Pavel Luzanov <p.luzanov@postgrespro.ru>
Re: psql: \dl+ to list large objects privileges Daniel Gustafsson <daniel@yesql.se>
Re: psql: \dl+ to list large objects privileges Pavel Luzanov <p.luzanov@postgrespro.ru>
Re: psql: \dl+ to list large objects privileges Georgios Kokolatos <gkokolatos@protonmail.com>
Re: psql: \dl+ to list large objects privileges Georgios Kokolatos <gkokolatos@protonmail.com>
Re: psql: \dl+ to list large objects privileges Pavel Luzanov <p.luzanov@postgrespro.ru>
Re: psql: \dl+ to list large objects privileges Pavel Luzanov <p.luzanov@postgrespro.ru>
Re: psql: \dl+ to list large objects privileges Pavel Luzanov <p.luzanov@postgrespro.ru>
Re: psql: \dl+ to list large objects privileges gkokolatos@pm.me
Re: psql: \dl+ to list large objects privileges Pavel Luzanov <p.luzanov@postgrespro.ru>
Re: psql: \dl+ to list large objects privileges Neil Chen <carpenter.nail.cz@gmail.com>
Re: psql: \dl+ to list large objects privileges Pavel Luzanov <p.luzanov@postgrespro.ru>
Re: psql: \dl+ to list large objects privileges Pavel Luzanov <p.luzanov@postgrespro.ru>
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
Hi,
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
changes.
On a high level I will recommend the addition of tests. There are similar tests
present in:
./src/test/regress/sql/psql.sql
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 = ...
break;
default:
status = PSQL_CMD_UNKNOWN;
break;
}
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);
Thoughts?
Cheers,
//GeorgiosВ списке pgsql-hackers по дате отправления