Re: tab completion of enum values is broken - Mailing list pgsql-hackers

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

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 .= "\\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 = 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 the
+ * user off to the right start.
+ */
 #define COMPLETE_WITH_ENUM_VALUE(type) \
 do { \
     char   *_completion_schema; \
     char   *_completion_type; \
+    bool    use_quotes; \
 \
     _completion_schema = strtokx(type, " \t\n\r", ".", "\"", 0, \
                                  false, false, pset.encoding); \
     (void) strtokx(NULL, " \t\n\r", ".", "\"", 0, \
                    false, false, pset.encoding); \
     _completion_type = strtokx(NULL, " \t\n\r", ".", "\"", 0, \
-                               false, false, pset.encoding);  \
-    if (_completion_type == NULL)\
+                               false, false, pset.encoding); \
+    use_quotes = (text[0] == '\'' || \
+                  start == 0 || rl_line_buffer[start - 1] != '\''); \
+    if (_completion_type == NULL) \
     { \
-        completion_charp = Query_for_list_of_enum_values; \
+        if (use_quotes) \
+            completion_charp = Query_for_list_of_enum_values_quoted; \
+        else \
+            completion_charp = Query_for_list_of_enum_values_unquoted; \
         completion_info_charp = type; \
     } \
     else \
     { \
-        completion_charp = Query_for_list_of_enum_values_with_schema; \
+        if (use_quotes) \
+            completion_charp = Query_for_list_of_enum_values_with_schema_quoted; \
+        else \
+            completion_charp = Query_for_list_of_enum_values_with_schema_unquoted; \
         completion_info_charp = _completion_type; \
         completion_info_charp2 = _completion_schema; \
     } \
@@ -678,7 +693,7 @@ static const SchemaQuery Query_for_list_of_collations = {
 "   AND (pg_catalog.quote_ident(nspname)='%s' "\
 "        OR '\"' || nspname || '\"' ='%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 = e.enumtypid "\
@@ -687,7 +702,16 @@ static const SchemaQuery Query_for_list_of_collations = {
 "        OR '\"' || typname || '\"'='%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 = e.enumtypid "\
+"   AND substring(enumlabel,1,%d)='%s' "\
+"   AND (pg_catalog.quote_ident(typname)='%s' "\
+"        OR '\"' || typname || '\"'='%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_namespace n "\
 " WHERE t.oid = e.enumtypid "\
@@ -698,6 +722,17 @@ static const SchemaQuery Query_for_list_of_collations = {
 "   AND (pg_catalog.quote_ident(nspname)='%s' "\
 "        OR '\"' || nspname || '\"' ='%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_namespace n "\
+" WHERE t.oid = e.enumtypid "\
+"   AND n.oid = t.typnamespace "\
+"   AND substring(enumlabel,1,%d)='%s' "\
+"   AND (pg_catalog.quote_ident(typname)='%s' "\
+"        OR '\"' || typname || '\"'='%s') "\
+"   AND (pg_catalog.quote_ident(nspname)='%s' "\
+"        OR '\"' || nspname || '\"' ='%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 end)
     if (matches == 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 = '\0';
+#endif
+#ifdef HAVE_RL_COMPLETION_SUPPRESS_QUOTE
+        rl_completion_suppress_quote = 1;
 #endif
     }