From: | Corey Huinker <corey(dot)huinker(at)gmail(dot)com> |
---|---|
To: | Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com> |
Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: \describe* |
Date: | 2019-03-09 00:19:45 |
Message-ID: | CADkLM=eX87dtGr-xHOmMaNhd6kcMU=tmekf2fZcoVRLiu2fUDw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Mar 4, 2019 at 1:45 PM Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
wrote:
>
>>> - 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
>>> ...
>>>
>>
> I just confirmed that there isn't tab completion for the existing S/+
> options, so it's hard to justify them for the equivalent verbose suffixes.
>
We can add completions for describe[-thing-]-verbose, but the
auto-completions start to run into combinatoric complexity, and the
original short-codes don't do that completion, probably for the same reason.
+ success =
>>> listTables("tvmsE", NULL, show_verbose, show_system);
>>> + }
>>> + status =
>>> PSQL_CMD_UNKNOWN;
>>>
>>>
> I'll look into this, thanks!
>
This was fixed, good find.
> - 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.
>>>
>>
I've changed the code to first strip out 0-1 instances of "-verbose" and
"-system" and the remaining string must be an exact match of a describe
command or it's an error. This same system could be applied to the short
commands to strip out 'S' and '+' and it might clean up the original code a
bit.
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)
>>>
>>
Same issue, same fix.
>>> 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.
>>>
>>
> There's some problems with a hash table. The function signatures vary
> quite a lot, and some require additional psql_scan_slash_options to be
> called. The hash option, if implemented, probably should be expanded to all
> slash commands, at which point maybe it belongs in psqlscanslash.l...
>
As I suspected, there's a lot of variance in the function signatures of the
various listSomething()/describeSomething() commands,
and listDbRoleSettings requires a second pattern to be scanned, and as far
as I know PsqlScanState isn't known inside describe.h, so building and
using a hash table would be a lot of work for uncertain gain. The original
code just plows through strings in alphabetical order, breaking things up
by comparing leading characters, so I largely did the same at the
des/decribe levels.
Instead of a hash table, It might be fun to write something that takes a
list of alphabetized strings, and builds a binary search tree at compile
time, but that would only work for the long form commands, the short forms
that allow filters like df[anptw]+ and d[tvmisE]+ effectively defeat any
attempt at hashing or btree-ing that I can presently imagine.
Having said that, here's v3 of the patch.
Since this is now waiting for v13, there's a bit more time to entertain the
question of whether we'd rather have these in psql or in a new server
command DESCRIBE [verbose] [system], and if so, whether the output of that
would itself be query-able or not.
Attachment | Content-Type | Size |
---|---|---|
0001-Add-describe-commands-to-compliment-d-commands.patch | text/x-patch | 63.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2019-03-09 01:09:20 | Re: Ordered Partitioned Table Scans |
Previous Message | David Rowley | 2019-03-09 00:18:57 | Re: Performance issue in foreign-key-aware join estimation |