Re: \describe*

From: Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
Subject: Re: \describe*
Date: 2019-03-04 14:01:24
Message-ID: 155170808403.16480.4991700744582309119.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2019-03-04 14:01:46 Re: Online verification of checksums
Previous Message Derek Hans 2019-03-04 14:00:50 Re: Update does not move row across foreign partitions in v11