Re: Psql patch to show access methods info

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Sergey Cherkashin <s(dot)cherkashin(at)postgrespro(dot)ru>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Psql patch to show access methods info
Date: 2018-11-29 06:47:41
Message-ID: 20181129064741.GB2356@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 23, 2018 at 05:13:24PM +0300, Sergey Cherkashin wrote:
> The attached patches are applied sequentially: first 0003-
> psql_add_am_info.patch, then 0003-psql_add_index_info.patch.

Thanks for doing a split. I have been looking at add_am to being with,
which is the first one in the set.

+ char *pattern2 =
psql_scan_slash_option(scan_state, OT_NORMAL, NULL, true);

The set of meta commands with a one-one mapping with the system catalogs
looks sensible to me, one suggestion I have would be to consider the
verbose option of all commands:
- \dAfp could have the strategy, purpose and sort purpose in its verbose
part.
- \dAfp could move the proc name with its arguments to the verbose
portion. I would imagine that removing the arguments could make sense.
- Is \dAf really useful as \dAfp actually proposes all the information
that really matters? And \dAfp joins with pg_opfamily.
- default and stored type could be moved to the verbose output of
\dAoc.

The columns names from \dAp could be better. What does "Can multi col"
mean? Well that's index support for multiple columns but that's rather
unclear for the user, no?

Wouldn't it be cleaner here to set the second pattern only if the first
pattern is defined?

+-- check printing info about access methods
+\dA
+List of access method
Regression tests are good for psql with deterministic matching patterns,
but I am not much a fan of things which print global results as they
result in more potential failures, and actually noise at the end. All
the tests checking unexisting patterns don't bring much either I think.

+ command name, each operator family is listed with it's owner.
s/it's/its/.

tab-complete.c:463:26: warning: ‘Query_for_list_of_operator_families’
defined but not used [-Wunused-const-variable=]
static const SchemaQuery Query_for_list_of_operator_families = {
Compiler complains.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Rishabh jain 2018-11-29 06:57:09 Application for mentor
Previous Message Tom Lane 2018-11-29 06:06:18 Re: pg_config wrongly marked as not parallel safe?