Date: Fri, 8 Oct 2021 07:01:23 +0000
From: David Fetter <david@fetter.org>
To: Edmund Horner <ejrh00@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: PATCH: psql tab completion for SELECT
Message-ID: <20211008070123.GN18391@fetter.org>
References: 
 <CAMyN-kB_xrU4iYdcF1j=tijgO1DSyjtb3j96O4UEj91XZrZcMg@mail.gmail.com>
 <CAMyN-kAtgYx7gshnhbrVu8cmcontrj2rV_hXMmZmfP4v2CrW7Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="RMedoP2+Pr6Rq0N2"
Content-Disposition: inline
In-Reply-To: 
 <CAMyN-kAtgYx7gshnhbrVu8cmcontrj2rV_hXMmZmfP4v2CrW7Q@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20211008070123.GN18391%40fetter.org>
Precedence: bulk


--RMedoP2+Pr6Rq0N2
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Sep 22, 2018 at 06:53:55PM +1200, Edmund Horner wrote:
> Hi all,
> 
> Here are some rebased versions of the last two patches.  No changes in
> functionality, except a minor case sensitivity fix in the "completion
> after commas" patch.
> 
> Edmund

I've rebased and updated these to be more principled about what
functions could be tab completed.

Still missing: tests.

What precisely is this supposed to do?

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

--RMedoP2+Pr6Rq0N2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v9-0001-Infrastructure-for-tab-completion-in-the-SELECT-l.patch"

From 87fb63d70f5e08f853a2b8aa94e03c0d665f5000 Mon Sep 17 00:00:00 2001
From: David Fetter <david.fetter@aiven.io>
Date: Thu, 7 Oct 2021 15:59:06 -0700
Subject: [PATCH v9 1/2] Infrastructure for tab completion in the SELECT list

---
 src/bin/psql/tab-complete.c | 65 ++++++++++++++++++++++++++++++-------
 1 file changed, 54 insertions(+), 11 deletions(-)

diff --git src/bin/psql/tab-complete.c src/bin/psql/tab-complete.c
index ecae9df8ed..5db143523b 100644
--- src/bin/psql/tab-complete.c
+++ src/bin/psql/tab-complete.c
@@ -1017,6 +1017,45 @@ static const VersionedQuery Query_for_list_of_subscriptions[] = {
 	{0, NULL}
 };
 
+static const SchemaQuery Query_for_list_of_selectable_functions[] = {
+	{
+		/* min_server_version */
+		70400,
+		/* catname */
+		"pg_catalog.pg_proc p",
+		/* Disallow functions which take or return pseudotypes which are other than all* or *record */
+		"NOT((ARRAY[p.prorettype] || coalesce(p.proallargtypes, p.proargtypes)) "
+		"    && ARRAY(SELECT oid FROM pg_catalog.pg_type WHERE typtype='p' AND typname !~ '(^all|record$)'))"
+		/* Disallow stored procedures XXX this may change later. */
+		" AND p.prokind <> 'p'"
+		/* viscondition */
+		"pg_catalog.pg_function_is_visible(p.oid)",
+		/* namespace */
+		"p.pronamespace",
+		/* result */
+		"pg_catalog.quote_ident(p.proname)||'('",
+		/* qualresult */
+		NULL
+	},
+	{0, NULL}
+};
+
+/*
+ * This addon is used to find (unqualified) column names to include
+ * alongside the function names from the query above.
+ */
+static const VersionedQuery Query_addon_for_list_of_selectable_attributes[] = {
+	{70400,
+		"    UNION ALL "
+		"   SELECT DISTINCT pg_catalog.quote_ident(attname) "
+		"     FROM pg_catalog.pg_attribute "
+		"    WHERE attnum > 0 "
+		"      AND NOT attisdropped "
+		"      AND substring(pg_catalog.quote_ident(attname),1,%d)='%s'"
+	},
+	{0, NULL}
+};
+
 /*
  * This is a list of all "things" in Pgsql, which can show up after CREATE or
  * DROP; and there is also a query to get a list of them.
@@ -3768,7 +3807,9 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH("IS");
 
 /* SELECT */
-	/* naah . . . */
+	else if (TailMatches("SELECT") || TailMatches("SELECT", "ALL|DISTINCT"))
+		COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_selectable_functions,
+											 Query_addon_for_list_of_selectable_attributes);
 
 /* SET, RESET, SHOW */
 	/* Complete with a variable name */
@@ -4432,7 +4473,8 @@ complete_from_versioned_schema_query(const char *text, int state)
  * where %d is the string length of the text and %s the text itself.
  *
  * If both simple_query and schema_query are non-NULL, then we construct
- * a schema query and append the (uninterpreted) string simple_query to it.
+ * a schema query and append the simple_query to it, replacing the %d and %s
+ * as described above.
  *
  * It is assumed that strings should be escaped to become SQL literals
  * (that is, what is in the query is actually ... '%s' ...)
@@ -4579,21 +4621,22 @@ _complete_from_query(const char *simple_query,
 							  " WHERE substring(pg_catalog.quote_ident(nspname) || '.',1,%d) ="
 							  " substring('%s',1,pg_catalog.length(pg_catalog.quote_ident(nspname))+1)) = 1",
 							  char_length, e_text);
-
-			/* If an addon query was provided, use it */
-			if (simple_query)
-				appendPQExpBuffer(&query_buffer, "\n%s", simple_query);
 		}
 		else
 		{
 			Assert(simple_query);
-			/* simple_query is an sprintf-style format string */
-			appendPQExpBuffer(&query_buffer, simple_query,
-							  char_length, e_text,
-							  e_info_charp, e_info_charp,
-							  e_info_charp2, e_info_charp2);
 		}
 
+		/*
+		 * simple_query is an sprintf-style format string (or it could be NULL, but
+		 * only if this is a schema query with no addon).
+		 */
+		if (simple_query)
+			appendPQExpBuffer(&query_buffer, simple_query,
+					  char_length, e_text,
+					  e_info_charp, e_info_charp,
+					  e_info_charp2, e_info_charp2);
+
 		/* Limit the number of records in the result */
 		appendPQExpBuffer(&query_buffer, "\nLIMIT %d",
 						  completion_max_records);
-- 
2.31.1


--RMedoP2+Pr6Rq0N2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v9-0002-SELECT-completion-after-commas.patch"

From 3764b9fe211db1fa322fa83103d39356928942ae Mon Sep 17 00:00:00 2001
From: David Fetter <david.fetter@aiven.io>
Date: Thu, 7 Oct 2021 18:56:52 -0700
Subject: [PATCH v9 2/2] SELECT completion after commas

---
 src/bin/psql/tab-complete.c | 64 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git src/bin/psql/tab-complete.c src/bin/psql/tab-complete.c
index 5db143523b..6fa4af08e0 100644
--- src/bin/psql/tab-complete.c
+++ src/bin/psql/tab-complete.c
@@ -247,6 +247,13 @@ do { \
 	COMPLETE_WITH_LIST(list); \
 } while (0)
 
+#define COMPLETE_CURRENT_WORD() \
+do { \
+	completion_case_sensitive = true; \
+	completion_charp = text; \
+	matches = rl_completion_matches(text, complete_from_const); \
+} while (0)
+
 #define COMPLETE_WITH_CS(...) \
 do { \
 	static const char *const list[] = { __VA_ARGS__, NULL }; \
@@ -1459,6 +1466,40 @@ ends_with(const char *s, char c)
 	return (length > 0 && s[length - 1] == c);
 }
 
+/*
+ * Get the last keyword matching a pattern.
+ */
+static const char *
+last_keyword(int previous_words_count, char **previous_words,
+			 const char *keyword_pattern)
+{
+	for (int i = 0; i < previous_words_count; i++)
+	{
+		if (word_matches(keyword_pattern, previous_words[i], false))
+			return previous_words[i];
+	}
+
+	return NULL;
+}
+
+/*
+ * Was the last keyword one of the expected ones?  For example,
+ * LastKeywordMatchesImpl(...,
+ * 						  "SELECT|FROM|WHERE|GROUP|ORDER", "SELECT")
+ * is true if SELECT is the most recent of those keywords to appear.
+ */
+static bool
+LastKeywordMatchesImpl(int previous_words_count, char **previous_words,
+					   const char *keyword_pattern,
+					   const char *accepted_pattern)
+{
+	const char *last_kw = last_keyword(previous_words_count, previous_words,
+									   keyword_pattern);
+	if (!last_kw)
+		return false;
+	return word_matches(last_kw, accepted_pattern, false);
+}
+
 /*
  * The completion function.
  *
@@ -1529,6 +1570,18 @@ psql_completion(const char *text, int start, int end)
 	HeadMatchesImpl(true, previous_words_count, previous_words, \
 					VA_ARGS_NARGS(__VA_ARGS__), __VA_ARGS__)
 
+	/* Macro for matching last keyword, case-insensitively. */
+#define LastKeywordMatches(keywords, pattern) \
+	(LastKeywordMatchesImpl(previous_words_count, previous_words, \
+							keywords, pattern))
+
+	/*
+	 * Macro for determining (loosely) which part of a DML query we are
+	 * currently in.
+	 */
+#define CurrentQueryPartMatches(pattern) \
+	(LastKeywordMatches("SELECT|INSERT|UPDATE|DELETE|FROM|WHERE|GROUP|ORDER|HAVING", pattern))
+
 	/* Known command-starting keywords. */
 	static const char *const sql_commands[] = {
 		"ABORT", "ALTER", "ANALYZE", "BEGIN", "CALL", "CHECKPOINT", "CLOSE", "CLUSTER",
@@ -1622,6 +1675,13 @@ psql_completion(const char *text, int start, int end)
 			matches = complete_from_variables(text, ":", "", true);
 	}
 
+	/*
+	 * If current word ends with a comma, add a space; this will expedite
+	 * completions after commas for SELECT, etc.
+	 */
+	if (ends_with(text, ','))
+		COMPLETE_CURRENT_WORD();
+
 	/* If no previous word, suggest one of the basic sql commands */
 	else if (previous_words_count == 0)
 		COMPLETE_WITH_LIST(sql_commands);
@@ -3807,7 +3867,9 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH("IS");
 
 /* SELECT */
-	else if (TailMatches("SELECT") || TailMatches("SELECT", "ALL|DISTINCT"))
+	else if (HeadMatches("SELECT|WITH") &&
+			 CurrentQueryPartMatches("SELECT") &&
+			 (ends_with(prev_wd, ',') || TailMatches("SELECT|ALL|DISTINCT")))
 		COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_selectable_functions,
 											 Query_addon_for_list_of_selectable_attributes);
 
-- 
2.31.1


--RMedoP2+Pr6Rq0N2--