From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: tab completion of enum values is broken
In-reply-to: <3213752.1642102872@sss.pgh.pa.us>
References: <8ca82d89-ec3d-8b28-8291-500efaf23b25@enterprisedb.com>
 <3213752.1642102872@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Thu, 13 Jan 2022 14:41:12 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <3430492.1642200496.0@sss.pgh.pa.us>
Date: Fri, 14 Jan 2022 17:48:39 -0500
Message-ID: <3430513.1642200519@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3430513.1642200519%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3430492.1642200496.1@sss.pgh.pa.us>

I wrote:
> Oh ... experimenting on macOS (with the system-provided libedit)
> shows no bug here.  So I guess we'll need to make this conditional
> somehow, perhaps on USE_FILENAME_QUOTING_FUNCTIONS.  That's another
> reason for not going overboard.

After further fooling with that, I concluded that the only workable
solution is a run-time check for whether the readline library included
the leading quote in what it hands us.  A big advantage of doing it this
way is that it mostly fixes my complaint #1: by crafting the check
properly, we will include quotes if the user hits TAB without having typed
anything, and we won't complete an incorrectly non-quoted identifier.
There's still nothing to be done about single quotes inside an enum
label, but I'm okay with blowing that case off.

So I think the attached is committable.  I've tried it on readline
7.0 (RHEL8) as well as whatever libedit Apple is currently shipping.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="enum-completion-2.patch";
	charset="us-ascii"
Content-ID: <3430492.1642200496.2@sss.pgh.pa.us>
Content-Description: enum-completion-2.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab=
_completion.pl
index ba98b8190b..d3d1bd650e 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -42,7 +42,8 @@ $node->start;
 $node->safe_psql('postgres',
 	    "CREATE TABLE tab1 (f1 int, f2 text);\n"
 	  . "CREATE TABLE mytab123 (f1 int, f2 text);\n"
-	  . "CREATE TABLE mytab246 (f1 int, f2 text);\n");
+	  . "CREATE TABLE mytab246 (f1 int, f2 text);\n"
+	  . "CREATE TYPE enum1 AS ENUM ('foo', 'bar', 'baz');\n");
 =

 # Developers would not appreciate this test adding a bunch of junk to
 # their ~/.psql_history, so be sure to redirect history into a temp file.
@@ -223,6 +224,16 @@ check_completion(
 =

 clear_line();
 =

+# check enum label completion
+# some versions of readline/libedit require two tabs here, some only need=
 one
+# also, some versions will offer quotes, some will not
+check_completion(
+	"ALTER TYPE enum1 RENAME VALUE 'ba\t\t",
+	qr|'?bar'? +'?baz'?|,
+	"offer multiple enum choices");
+
+clear_line();
+
 # send psql an explicit \q to shut it down, else pty won't close properly
 $timer->start(5);
 $in .=3D "\\q\n";
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 39be6f556a..f7ce90da6e 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -278,25 +278,40 @@ do { \
 	matches =3D rl_completion_matches(text, complete_from_query); \
 } while (0)
 =

+/*
+ * libedit will typically include the literal's leading single quote in
+ * "text", while readline will not.  Adapt our offered strings to fit.
+ * If we don't yet have text, include quotes in what's offered, to get th=
e
+ * user off to the right start.
+ */
 #define COMPLETE_WITH_ENUM_VALUE(type) \
 do { \
 	char   *_completion_schema; \
 	char   *_completion_type; \
+	bool	use_quotes; \
 \
 	_completion_schema =3D strtokx(type, " \t\n\r", ".", "\"", 0, \
 								 false, false, pset.encoding); \
 	(void) strtokx(NULL, " \t\n\r", ".", "\"", 0, \
 				   false, false, pset.encoding); \
 	_completion_type =3D strtokx(NULL, " \t\n\r", ".", "\"", 0, \
-							   false, false, pset.encoding);  \
-	if (_completion_type =3D=3D NULL)\
+							   false, false, pset.encoding); \
+	use_quotes =3D (text[0] =3D=3D '\'' || \
+				  start =3D=3D 0 || rl_line_buffer[start - 1] !=3D '\''); \
+	if (_completion_type =3D=3D NULL) \
 	{ \
-		completion_charp =3D Query_for_list_of_enum_values; \
+		if (use_quotes) \
+			completion_charp =3D Query_for_list_of_enum_values_quoted; \
+		else \
+			completion_charp =3D Query_for_list_of_enum_values_unquoted; \
 		completion_info_charp =3D type; \
 	} \
 	else \
 	{ \
-		completion_charp =3D Query_for_list_of_enum_values_with_schema; \
+		if (use_quotes) \
+			completion_charp =3D Query_for_list_of_enum_values_with_schema_quoted;=
 \
+		else \
+			completion_charp =3D Query_for_list_of_enum_values_with_schema_unquote=
d; \
 		completion_info_charp =3D _completion_type; \
 		completion_info_charp2 =3D _completion_schema; \
 	} \
@@ -678,7 +693,7 @@ static const SchemaQuery Query_for_list_of_collations =
=3D {
 "   AND (pg_catalog.quote_ident(nspname)=3D'%s' "\
 "        OR '\"' || nspname || '\"' =3D'%s') "
 =

-#define Query_for_list_of_enum_values \
+#define Query_for_list_of_enum_values_quoted \
 "SELECT pg_catalog.quote_literal(enumlabel) "\
 "  FROM pg_catalog.pg_enum e, pg_catalog.pg_type t "\
 " WHERE t.oid =3D e.enumtypid "\
@@ -687,7 +702,16 @@ static const SchemaQuery Query_for_list_of_collations=
 =3D {
 "        OR '\"' || typname || '\"'=3D'%s') "\
 "   AND pg_catalog.pg_type_is_visible(t.oid)"
 =

-#define Query_for_list_of_enum_values_with_schema \
+#define Query_for_list_of_enum_values_unquoted \
+"SELECT enumlabel "\
+"  FROM pg_catalog.pg_enum e, pg_catalog.pg_type t "\
+" WHERE t.oid =3D e.enumtypid "\
+"   AND substring(enumlabel,1,%d)=3D'%s' "\
+"   AND (pg_catalog.quote_ident(typname)=3D'%s' "\
+"        OR '\"' || typname || '\"'=3D'%s') "\
+"   AND pg_catalog.pg_type_is_visible(t.oid)"
+
+#define Query_for_list_of_enum_values_with_schema_quoted \
 "SELECT pg_catalog.quote_literal(enumlabel) "\
 "  FROM pg_catalog.pg_enum e, pg_catalog.pg_type t, pg_catalog.pg_namespa=
ce n "\
 " WHERE t.oid =3D e.enumtypid "\
@@ -698,6 +722,17 @@ static const SchemaQuery Query_for_list_of_collations=
 =3D {
 "   AND (pg_catalog.quote_ident(nspname)=3D'%s' "\
 "        OR '\"' || nspname || '\"' =3D'%s') "
 =

+#define Query_for_list_of_enum_values_with_schema_unquoted \
+"SELECT enumlabel "\
+"  FROM pg_catalog.pg_enum e, pg_catalog.pg_type t, pg_catalog.pg_namespa=
ce n "\
+" WHERE t.oid =3D e.enumtypid "\
+"   AND n.oid =3D t.typnamespace "\
+"   AND substring(enumlabel,1,%d)=3D'%s' "\
+"   AND (pg_catalog.quote_ident(typname)=3D'%s' "\
+"        OR '\"' || typname || '\"'=3D'%s') "\
+"   AND (pg_catalog.quote_ident(nspname)=3D'%s' "\
+"        OR '\"' || nspname || '\"' =3D'%s') "
+
 #define Query_for_list_of_template_databases \
 "SELECT pg_catalog.quote_ident(d.datname) "\
 "  FROM pg_catalog.pg_database d "\
@@ -4413,8 +4448,12 @@ psql_completion(const char *text, int start, int en=
d)
 	if (matches =3D=3D NULL)
 	{
 		COMPLETE_WITH_CONST(true, "");
+		/* Also, prevent Readline from appending stuff to the non-match */
 #ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER
 		rl_completion_append_character =3D '\0';
+#endif
+#ifdef HAVE_RL_COMPLETION_SUPPRESS_QUOTE
+		rl_completion_suppress_quote =3D 1;
 #endif
 	}
 =


------- =_aaaaaaaaaa0--