Further tweaks for psql's new tab-completion logic.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 9 Feb 2022 22:06:21 +0000 (17:06 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 9 Feb 2022 22:06:21 +0000 (17:06 -0500)
The behavior I proposed, of matching case only when only keywords
are available to complete, turns out to be too cute.  It adds about
as many problems as it removes.  Simplify down to ilmari's original
proposal of just always matching case when completing a keyword.

Also, I noticed while testing this that we've pessimized the behavior
for qualified GUC names: the code is insisting that they be
double-quoted, which was not the case before.  Fix that by treating
GUC names as verbatim matches instead of possibly-schema-qualified
names.  (While it's tempting to try to split qualified GUC names
so that we *could* treat them with the schema-qualified-name code
path, that really isn't going to work in light of guc.c's willingness
to allow more than two name components.)

Dagfinn Ilmari MannsÃ¥ker and Tom Lane

Discussion: https://postgr.es/m/445692.1644018081@sss.pgh.pa.us

src/bin/psql/t/010_tab_completion.pl
src/bin/psql/tab-complete.c

index 569dd442e8ad0acf0d6f91acc6469b519cd977e9..005961f34d4ceb56e14f59392b5d7d073f0cd347 100644 (file)
@@ -339,8 +339,7 @@ check_completion(
 clear_line();
 
 # check completion of a keyword offered in addition to object names;
-# such a keyword should obey COMP_KEYWORD_CASE once only keyword
-# completions are possible
+# such a keyword should obey COMP_KEYWORD_CASE
 foreach (
    [ 'lower',          'CO', 'column' ],
    [ 'upper',          'co', 'COLUMN' ],
@@ -353,10 +352,6 @@ foreach (
        "\\set COMP_KEYWORD_CASE $case\n",
        qr/postgres=#/,
        "set completion case to '$case'");
-   check_completion("alter table tab1 rename c\t\t",
-       qr|COLUMN|,
-       "offer keyword COLUMN for input c<TAB>, COMP_KEYWORD_CASE = $case");
-   clear_query();
    check_completion("alter table tab1 rename $in\t\t\t",
        qr|$out|,
        "offer keyword $out for input $in<TAB>, COMP_KEYWORD_CASE = $case");
@@ -410,6 +405,23 @@ check_completion(" iso\t", qr/iso_8601 /, "complete a GUC enum value");
 
 clear_query();
 
+# same, for qualified GUC names
+check_completion(
+   "DO \$\$begin end\$\$ LANGUAGE plpgsql;\n",
+   qr/postgres=# /,
+   "load plpgsql extension");
+
+check_completion("set plpg\t", qr/plpg\a?sql\./,
+   "complete prefix of a GUC name");
+check_completion(
+   "var\t\t",
+   qr/variable_conflict TO/,
+   "complete a qualified GUC name");
+check_completion(" USE_C\t",
+   qr/use_column/, "complete a qualified GUC enum value");
+
+clear_query();
+
 # check completions for psql variables
 check_completion("\\set VERB\t", qr/VERBOSITY /,
    "complete a psql variable name");
index d1e421bc0f1f8716531474d2fe2d6aa25cc55388..25d3abbcf18ddd9dcf35d50f0f4133b2d30708c7 100644 (file)
@@ -257,13 +257,22 @@ do { \
 } while (0)
 
 #define COMPLETE_WITH_QUERY_VERBATIM(query) \
+   COMPLETE_WITH_QUERY_VERBATIM_LIST(query, NULL)
+
+#define COMPLETE_WITH_QUERY_VERBATIM_LIST(query, list) \
 do { \
    completion_charp = query; \
-   completion_charpp = NULL; \
+   completion_charpp = list; \
    completion_verbatim = true; \
    matches = rl_completion_matches(text, complete_from_query); \
 } while (0)
 
+#define COMPLETE_WITH_QUERY_VERBATIM_PLUS(query, ...) \
+do { \
+   static const char *const list[] = { __VA_ARGS__, NULL }; \
+   COMPLETE_WITH_QUERY_VERBATIM_LIST(query, list); \
+} while (0)
+
 #define COMPLETE_WITH_VERSIONED_QUERY(query) \
    COMPLETE_WITH_VERSIONED_QUERY_LIST(query, NULL)
 
@@ -1273,6 +1282,7 @@ static char *_complete_from_query(const char *simple_query,
                                  bool verbatim,
                                  const char *text, int state);
 static void set_completion_reference(const char *word);
+static void set_completion_reference_verbatim(const char *word);
 static char *complete_from_list(const char *text, int state);
 static char *complete_from_const(const char *text, int state);
 static void append_variable_names(char ***varnames, int *nvars,
@@ -2058,8 +2068,8 @@ psql_completion(const char *text, int start, int end)
    else if (Matches("ALTER", "SYSTEM"))
        COMPLETE_WITH("SET", "RESET");
    else if (Matches("ALTER", "SYSTEM", "SET|RESET"))
-       COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_alter_system_set_vars,
-                                "ALL");
+       COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_alter_system_set_vars,
+                                         "ALL");
    else if (Matches("ALTER", "SYSTEM", "SET", MatchAny))
        COMPLETE_WITH("TO");
    /* ALTER VIEW <name> */
@@ -4038,17 +4048,17 @@ psql_completion(const char *text, int start, int end)
 /* SET, RESET, SHOW */
    /* Complete with a variable name */
    else if (TailMatches("SET|RESET") && !TailMatches("UPDATE", MatchAny, "SET"))
-       COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_set_vars,
-                                "CONSTRAINTS",
-                                "TRANSACTION",
-                                "SESSION",
-                                "ROLE",
-                                "TABLESPACE",
-                                "ALL");
+       COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_set_vars,
+                                         "CONSTRAINTS",
+                                         "TRANSACTION",
+                                         "SESSION",
+                                         "ROLE",
+                                         "TABLESPACE",
+                                         "ALL");
    else if (Matches("SHOW"))
-       COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_show_vars,
-                                "SESSION AUTHORIZATION",
-                                "ALL");
+       COMPLETE_WITH_QUERY_VERBATIM_PLUS(Query_for_list_of_show_vars,
+                                         "SESSION AUTHORIZATION",
+                                         "ALL");
    else if (Matches("SHOW", "SESSION"))
        COMPLETE_WITH("AUTHORIZATION");
    /* Complete "SET TRANSACTION" */
@@ -4150,7 +4160,7 @@ psql_completion(const char *text, int start, int end)
            {
                if (strcmp(guctype, "enum") == 0)
                {
-                   set_completion_reference(prev2_wd);
+                   set_completion_reference_verbatim(prev2_wd);
                    COMPLETE_WITH_QUERY_PLUS(Query_for_values_of_enum_GUC,
                                             "DEFAULT");
                }
@@ -4707,7 +4717,7 @@ complete_from_versioned_schema_query(const char *text, int state)
  * version of the string provided in completion_ref_object.  If there is a
  * third '%s', it will be replaced by a suitably-escaped version of the string
  * provided in completion_ref_schema.  Those strings should be set up
- * by calling set_completion_reference().
+ * by calling set_completion_reference or set_completion_reference_verbatim.
  * Simple queries should return a single column of matches.  If "verbatim"
  * is true, the matches are returned as-is; otherwise, they are taken to
  * be SQL identifiers and quoted if necessary.
@@ -5037,11 +5047,7 @@ _complete_from_query(const char *simple_query,
                if (pg_strncasecmp(text, item, strlen(text)) == 0)
                {
                    num_keywords++;
-                   /* Match keyword case if we are returning only keywords */
-                   if (num_schema_only == 0 && num_query_other == 0)
-                       return pg_strdup_keyword_case(item, text);
-                   else
-                       return pg_strdup(item);
+                   return pg_strdup_keyword_case(item, text);
                }
            }
        }
@@ -5059,11 +5065,7 @@ _complete_from_query(const char *simple_query,
                if (pg_strncasecmp(text, item, strlen(text)) == 0)
                {
                    num_keywords++;
-                   /* Match keyword case if we are returning only keywords */
-                   if (num_schema_only == 0 && num_query_other == 0)
-                       return pg_strdup_keyword_case(item, text);
-                   else
-                       return pg_strdup(item);
+                   return pg_strdup_keyword_case(item, text);
                }
            }
        }
@@ -5100,6 +5102,17 @@ set_completion_reference(const char *word)
                     &schemaquoted, &objectquoted);
 }
 
+/*
+ * Set up completion_ref_object when it should just be
+ * the given word verbatim.
+ */
+static void
+set_completion_reference_verbatim(const char *word)
+{
+   completion_ref_schema = NULL;
+   completion_ref_object = pg_strdup(word);
+}
+
 
 /*
  * This function returns in order one of a fixed, NULL pointer terminated list