Re: \describe*

Поиск
Список
Период
Сортировка
От Ibrar Ahmed
Тема Re: \describe*
Дата
Msg-id CALtqXTdXo61W03cMLSGOZujjFguUYxhXAk=_vo_gB_wf1go-EQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: \describe*  (Ibrar Ahmed <ibrar.ahmad@gmail.com>)
Ответы Re: \describe*
Список pgsql-hackers
Hi Corey,

Here is the modified patch (sample).



On Mon, Mar 4, 2019 at 7:02 PM Ibrar Ahmed <ibrar.ahmad@gmail.com> wrote:
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

Thanks for the patch, I have reviewed the patch and have some comments about the patch. The review includes the testing of the patch along with some code review.

Here are my testings results,

- Tab completion for \descibe-verbose.
I know that \d+ tab completion is also not there, but I think we must have tab completion for \descibe-verbose.

postgres=# \describe-
\describe-extension                       \describe-replication-publication         \describe-user-mapping
\describe-foreign-data-wrapper            \describe-replication-subscription        \describe-view
\describe-foreign-server                  \describe-role                            \describe-window-function
\describe-foreign-table                   \describe-rule 
 ...


- Error message in each command.
There is an error message after each command, here is the example.
postgres=# \describe
        List of relations
 Schema | Name | Type  |  Owner 
--------+------+-------+---------
 public | foo  | table | vagrant

(1 row)
Invalid command \describe. Try \? for help.


I think this status is causing the problem.



+                                                               /* standard listing of interesting things */
+                                                               success = listTables("tvmsE", NULL, show_verbose, show_system);
+                                               }
+                                               status = PSQL_CMD_UNKNOWN;




- Confusion about \desc and \desC
There is confusion while running the \desc command. I know the problem, but the user may confuse by this.
postgres=# \desC
       List of foreign servers
 Name | Owner | Foreign-data wrapper
------+-------+----------------------
(0 rows)

postgres=# \desc
Invalid command \desc. Try \? for help.

- Auto-completion of commands.
There is some more confusion in the completion of commands.

This command shows List of aggregates.
postgres=# \describe-aggregate-function
                     List of aggregate functions
 Schema | Name | Result data type | Argument data types | Description
--------+------+------------------+---------------------+-------------
(0 rows)



This command shows a list of relation "\d"
postgres=# \describe-aggregatE-function
        List of relations
 Schema | Name | Type  |  Owner 
--------+------+-------+---------
 public | foo  | table | vagrant
(1 row)

This command also shows a list of relations "\d".
postgres=# \describe-aggr
        List of relations
 Schema | Name | Type  |  Owner 
--------+------+-------+---------
 public | foo  | table | vagrant
(1 row)

This command shows error messages.
postgres=# \descr
Invalid command \descr. Try \? for help.

...


Code review.
-------------

I have done a brief code review except for the documentation code. I don't like this code

if (cmd_match(cmd,"describe-aggregate-function"))                               
 success = describeAggregates(pattern, show_verbose, show_system);
                             else if (cmd_match(cmd, "describe-access-method"))
                                 success = describeAccessMethods(pattern, show_verbose);
                             else if (cmd_match(cmd, "describe-tablespace"))
                                 success = describeTablespaces(pattern, show_verbose);
                             else if (cmd_match(cmd, "describe-conversion"))
                                 success = listConversions(pattern, show_verbose, show_system);
                             else if (cmd_match(cmd, "describe-cast"))
                                 success = listCasts(pattern, show_verbose


This can be achieved with the list/array/hash table, so I have changed that code in the attached patch just for a sample if you want I can do that for whole code.

--
Ibrar Ahmed

The new status of this patch is: Waiting on Author


--
Ibrar Ahmed
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tomas Vondra
Дата:
Сообщение: Re: Online verification of checksums
Следующее
От: Antonin Houska
Дата:
Сообщение: Re: Ordered Partitioned Table Scans