Re: psql: Make tab completion work for ANALYZE VERBOSE ... - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: psql: Make tab completion work for ANALYZE VERBOSE ...
Date
Msg-id 49CC8680.3090904@enterprisedb.com
Whole thread Raw
In response to psql: Make tab completion work for ANALYZE VERBOSE ...  (Greg Sabino Mullane <greg@endpoint.com>)
Responses Re: psql: Make tab completion work for ANALYZE VERBOSE ...  (Greg Sabino Mullane <greg@endpoint.com>)
List pgsql-hackers
Greg Sabino Mullane wrote:
> Quick patch to fix the fact that the EXPLAIN ANALYZE VERBOSE is clobbering
> tab-completion for ANALYZE VERBOSE.

Thanks.

> *** tab-complete.c    24 Feb 2009 10:06:34 -0000    1.180
> --- tab-complete.c    27 Mar 2009 01:29:06 -0000
> ***************
> *** 1627,1633 ****
>       else if (pg_strcasecmp(prev_wd, "VERBOSE") == 0 &&
>                pg_strcasecmp(prev3_wd, "VACUUM") != 0 &&
>                pg_strcasecmp(prev4_wd, "VACUUM") != 0 &&
> !              (pg_strcasecmp(prev2_wd, "ANALYZE") == 0 ||
>                 pg_strcasecmp(prev2_wd, "EXPLAIN") == 0))
>       {
>           static const char *const list_EXPLAIN[] =
> --- 1627,1634 ----
>       else if (pg_strcasecmp(prev_wd, "VERBOSE") == 0 &&
>                pg_strcasecmp(prev3_wd, "VACUUM") != 0 &&
>                pg_strcasecmp(prev4_wd, "VACUUM") != 0 &&
> !              ((pg_strcasecmp(prev2_wd, "ANALYZE") == 0 && 
> !                pg_strcasecmp(prev3_wd, "EXPLAIN") == 0) ||
>                 pg_strcasecmp(prev2_wd, "EXPLAIN") == 0))
>       {
>           static const char *const list_EXPLAIN[] =

I find that that particular rule is formatted differently than the 
others. It took me a while to figure out how it works. All the others 
check the keywords from left to right, but this checks that the previous 
word is VERBOSE and works to the left from there, kind of. I also don't 
understand why the explicit check for VACUUM is there. It only makes a 
difference if you write something like "VACUUM EXPLAIN VERBOSE", which 
isn't valid. I guess it was needed before this fix to not match "VACUUM 
ANALYZE", but isn't anymore.

I'd suggest to write it like this:

>     else if ((pg_strcasecmp(prev2_wd, "EXPLAIN") == 0 &&
>               pg_strcasecmp(prev_wd, "VERBOSE") == 0) ||
>              (pg_strcasecmp(prev3_wd, "EXPLAIN") == 0 &&
>               pg_strcasecmp(prev2_wd, "ANALYZE") == 0 &&
>               pg_strcasecmp(prev_wd, "VERBOSE") == 0))


While we're at it, any idea what the logic behind this rule is:

>     else if ((pg_strcasecmp(prev_wd, "ANALYZE") == 0 &&
>               pg_strcasecmp(prev2_wd, "VERBOSE") == 0) ||
>              (pg_strcasecmp(prev_wd, "VERBOSE") == 0 &&
>               pg_strcasecmp(prev2_wd, "ANALYZE") == 0))
>         COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);

? The first part of that I understand, "ANALYZE VERBOSE", but "VERBOSE 
ANALYZE" isn't valid SQL.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


pgsql-hackers by date:

Previous
From: Andrew Gierth
Date:
Subject: Re: Any reason not to return row_count in cursor of plpgsql?
Next
From: Magnus Hagander
Date:
Subject: Re: 8.4 open items list