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
Date: Fri, 14 Jan 2022 17:48:39 -0500
Message-ID: <3430513.1642200519@sss.pgh.pa.us> (raw)
In-Reply-To: <3213752.1642102872@sss.pgh.pa.us>
References: <8ca82d89-ec3d-8b28-8291-500efaf23b25@enterprisedb.com>
<3213752.1642102872@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
Attachments:enum-completion-2.patch (text/x-diff, 5.2K) downloaddiff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.plindex 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.cindex 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
}
=
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, peter.eisentraut@enterprisedb.com
Subject: Re: tab completion of enum values is broken
In-Reply-To: <3430513.1642200519@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox