From: Tom Lane <tgl@sss.pgh.pa.us>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Arthur Zakirov <a.zakirov@postgrespro.ru>
Cc: Edmund Horner <ejrh00@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: adding tab completions
Date: Wed, 12 Sep 2018 15:37:09 -0400
Message-ID: <14255.1536781029@sss.pgh.pa.us> (raw)
In-Reply-To: <20180730082414.GA21163@zakirov.localdomain>
References: <CAMyN-kDUKKtaZRQJu_-ahr258b1P0PC341xyH0CK3CWi7fet7g@mail.gmail.com>
<20180529092726.GA6535@zakirov.localdomain>
<20180603231859.GV5164@telsasoft.com>
<20180604033922.GW5164@telsasoft.com>
<20180605142941.GA25460@zakirov.localdomain>
<20180609234212.GT10885@telsasoft.com>
<20180611203547.GA18675@artur-book.localdomain>
<20180627201051.GG3554@telsasoft.com>
<20180628112037.GA6937@zakirov.localdomain>
<20180730004243.GE3222@telsasoft.com>
<20180730082414.GA21163@zakirov.localdomain>
Arthur Zakirov <a.zakirov@postgrespro.ru> writes:
> On Sun, Jul 29, 2018 at 07:42:43PM -0500, Justin Pryzby wrote:
>>> Actually..another thought: since toast tables may be VACUUM-ed, should I
>>> introduce Query_for_list_of_tpmt ?
>> I didn't include this one yet though.
> I think it could be done by a separate patch.
I don't actually think that's a good idea. It's more likely to clutter
peoples' completion lists than offer anything they want. Even if someone
actually does want to vacuum a toast table, they are not likely to be
entering its name via tab completion; they're going to have identified
which table they want via some query, and then they'll be doing something
like copy-and-paste out of a query result.
I pushed the first three hunks of the current patch, since those seem
like pretty uncontroversial bug fixes for v11 issues. Attached is a
rebased patch for the remainder, with some very minor adjustments.
The main thing that is bothering me about the remainder is its desire
to offer single-punctuation-character completions such as "(". I do
not see the point of that. You can't select a completion without
typing at least one character, so what does it accomplish to offer
those options, except clutter?
BTW, the cfbot has been claiming that this CF item fails patch
application, but that seems to be because it's not actually testing
the most recent patch. I speculate that that's because you did not
name the attachment "something.patch" or "something.diff". Please
use a more conventional filename for future attachments.
regards, tom lane
Attachments:tab-completions-12.patch (text/x-diff, 6.6K) downloaddiff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.cindex 7549b40..69e6632 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(const char *text, int st
*** 2802,2815 ****
else if (Matches1("EXECUTE"))
COMPLETE_WITH_QUERY(Query_for_list_of_prepared_statements);
! /* EXPLAIN */
!
! /*
! * Complete EXPLAIN [ANALYZE] [VERBOSE] with list of EXPLAIN-able commands
! */
else if (Matches1("EXPLAIN"))
! COMPLETE_WITH_LIST7("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE",
! "ANALYZE", "VERBOSE");
else if (Matches2("EXPLAIN", "ANALYZE"))
COMPLETE_WITH_LIST6("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE",
"VERBOSE");
--- 2802,2834 ----
else if (Matches1("EXECUTE"))
COMPLETE_WITH_QUERY(Query_for_list_of_prepared_statements);
! /*
! * EXPLAIN [ ( option [, ...] ) ] statement
! * EXPLAIN [ ANALYZE ] [ VERBOSE ] statement
! */
else if (Matches1("EXPLAIN"))
! COMPLETE_WITH_LIST8("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE",
! "ANALYZE", "VERBOSE", "(");
! else if (HeadMatches2("EXPLAIN", "("))
! {
! if (ends_with(prev_wd, '(') || ends_with(prev_wd, ','))
! COMPLETE_WITH_LIST7("ANALYZE", "VERBOSE", "COSTS", "BUFFERS",
! "TIMING", "SUMMARY", "FORMAT");
! else if (TailMatches1("FORMAT"))
! COMPLETE_WITH_LIST4("TEXT", "XML", "JSON", "YAML");
! else if (TailMatches1("ANALYZE|VERBOSE|COSTS|BUFFERS|TIMING|SUMMARY"))
! COMPLETE_WITH_LIST4(",", ")", "ON", "OFF");
! else
! COMPLETE_WITH_LIST2(",", ")");
! }
! else if (Matches2("EXPLAIN", MatchAny) && ends_with(prev_wd, ')'))
! {
! /*
! * get_previous_words treats a parenthesized option list as one word,
! * so the above test is correct.
! */
! COMPLETE_WITH_LIST5("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE");
! }
else if (Matches2("EXPLAIN", "ANALYZE"))
COMPLETE_WITH_LIST6("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE",
"VERBOSE");
*************** psql_completion(const char *text, int st
*** 3369,3401 ****
COMPLETE_WITH_CONST("OPTIONS");
/*
! * VACUUM [ FULL | FREEZE ] [ VERBOSE ] [ table ]
! * VACUUM [ FULL | FREEZE ] [ VERBOSE ] ANALYZE [ table [ (column [, ...] ) ] ]
*/
else if (Matches1("VACUUM"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
" UNION SELECT 'FULL'"
" UNION SELECT 'FREEZE'"
" UNION SELECT 'ANALYZE'"
" UNION SELECT 'VERBOSE'");
! else if (Matches2("VACUUM", "FULL|FREEZE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
" UNION SELECT 'ANALYZE'"
" UNION SELECT 'VERBOSE'");
! else if (Matches3("VACUUM", "FULL|FREEZE", "ANALYZE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
! " UNION SELECT 'VERBOSE'");
! else if (Matches3("VACUUM", "FULL|FREEZE", "VERBOSE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
" UNION SELECT 'ANALYZE'");
! else if (Matches2("VACUUM", "VERBOSE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
" UNION SELECT 'ANALYZE'");
! else if (Matches2("VACUUM", "ANALYZE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm,
! " UNION SELECT 'VERBOSE'");
else if (HeadMatches1("VACUUM"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tm, NULL);
/* WITH [RECURSIVE] */
--- 3388,3435 ----
COMPLETE_WITH_CONST("OPTIONS");
/*
! * VACUUM [ ( option [, ...] ) ] [ table_and_columns [, ...] ]
! * VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ table_and_columns [, ...] ]
*/
else if (Matches1("VACUUM"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpm,
" UNION SELECT 'FULL'"
" UNION SELECT 'FREEZE'"
" UNION SELECT 'ANALYZE'"
" UNION SELECT 'VERBOSE'");
! else if (Matches2("VACUUM", "FULL"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpm,
! " UNION SELECT 'FREEZE'"
" UNION SELECT 'ANALYZE'"
" UNION SELECT 'VERBOSE'");
! else if (Matches2("VACUUM", "FULL|FREEZE") ||
! Matches3("VACUUM", "FULL", "FREEZE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpm,
! " UNION SELECT 'VERBOSE'"
" UNION SELECT 'ANALYZE'");
! else if (Matches2("VACUUM", "VERBOSE") ||
! Matches3("VACUUM", "FULL|FREEZE", "VERBOSE") ||
! Matches4("VACUUM", "FULL", "FREEZE", "VERBOSE"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpm,
" UNION SELECT 'ANALYZE'");
! else if (HeadMatches2("VACUUM", "("))
! {
! if (ends_with(prev_wd, ',') || ends_with(prev_wd, '('))
! COMPLETE_WITH_LIST5("FULL", "FREEZE", "ANALYZE", "VERBOSE", "DISABLE_PAGE_SKIPPING");
! else
! COMPLETE_WITH_LIST2(",", ")");
! }
! else if (HeadMatches1("VACUUM") && TailMatches1("("))
! /* "VACUUM (" should be caught above */
! COMPLETE_WITH_ATTR(prev2_wd, "");
! else if (HeadMatches2("VACUUM", MatchAny) && !ends_with(prev_wd, ',') &&
! !TailMatches1("ANALYZE") &&
! !(previous_words_count == 2 && prev_wd[0] == '(' && ends_with(prev_wd, ')')))
! /* Comma to support vacuuming multiple tables */
! /* Parens to support analyzing a partial column list */
! COMPLETE_WITH_LIST3(",", "(", ")");
else if (HeadMatches1("VACUUM"))
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tpm, "");
/* WITH [RECURSIVE] */
*************** psql_completion(const char *text, int st
*** 3406,3414 ****
else if (Matches1("WITH"))
COMPLETE_WITH_CONST("RECURSIVE");
! /* ANALYZE */
! /* Complete with list of tables */
else if (Matches1("ANALYZE"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tmf, NULL);
/* WHERE */
--- 3440,3467 ----
else if (Matches1("WITH"))
COMPLETE_WITH_CONST("RECURSIVE");
! /*
! * ANALYZE [ ( option [, ...] ) ] [ table_and_columns [, ...] ]
! * ANALYZE [ VERBOSE ] [ table_and_columns [, ...] ]
! */
! else if (Matches2("ANALYZE", "("))
! COMPLETE_WITH_CONST("VERBOSE)");
else if (Matches1("ANALYZE"))
+ COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tmf,+ " UNION SELECT 'VERBOSE'"+ " UNION SELECT '('"+ );+ else if (HeadMatches1("ANALYZE") && TailMatches1("("))+ /* "ANALYZE (" should be caught above */+ COMPLETE_WITH_ATTR(prev2_wd, "");+ else if (HeadMatches2("ANALYZE", MatchAny) &&+ !ends_with(prev_wd, ',') &&+ !(previous_words_count == 2 && prev_wd[0] == '(' && ends_with(prev_wd, ')')) &&+ !TailMatches1("VERBOSE"))+ /* Support analyze of multiple tables */+ /* or analyze table(column1, column2) */+ COMPLETE_WITH_LIST3(",", "(", ")");+ else if (HeadMatches1("ANALYZE"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tmf, NULL);
/* WHERE */
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pryzby@telsasoft.com, a.zakirov@postgrespro.ru, ejrh00@gmail.com
Subject: Re: adding tab completions
In-Reply-To: <14255.1536781029@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox