Further tweaks for psql's new tab-completion logic.

tglsfdc · tglsfdc · commit f0cd9097cfac · 2022-02-09T17:06:21.000-05:00
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
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
@@ -339,8 +339,7 @@ sub clear_line
 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 @@ sub clear_line
 		"\\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 @@ sub clear_line
 
 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");
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
@@ -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
