Re: adding tab completions - Mailing list pgsql-hackers

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

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 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 */