Hi Corey, Here is the modified patch (sample).
On Mon, Mar 4, 2019 at 7:02 PM Ibrar Ahmed <ibrar.ah...@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
0001-Add-describe-commands-to-compliment-d-commands-ibrar-v2.patch
Description: Binary data
