tab completion of enum values is broken
This doesn't work anymore:
create type e2 as enum ('foo', 'bar');
alter type e2 rename value 'b<TAB>
This now results in
alter type e2 rename value 'b'
Bisecting blames
commit cd69ec66c88633c09bc9a984a7f0930e09c7c96e
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu Jan 23 11:07:12 2020 -0500
Improve psql's tab completion for filenames.
which did deal with quoting of things to be completed, so it seems very
plausible to be some collateral damage.
The queries issued by the completion engine are
bad
LOG: statement: SELECT pg_catalog.quote_literal(enumlabel) FROM
pg_catalog.pg_enum e, pg_catalog.pg_type t WHERE t.oid = e.enumtypid
AND substring(pg_catalog.quote_literal(enumlabel),1,1)='b' AND
(pg_catalog.quote_ident(typname)='e2' OR '"' || typname ||
'"'='e2') AND pg_catalog.pg_type_is_visible(t.oid) LIMIT 1000
good
LOG: statement: SELECT pg_catalog.quote_literal(enumlabel) FROM
pg_catalog.pg_enum e, pg_catalog.pg_type t WHERE t.oid = e.enumtypid
AND substring(pg_catalog.quote_literal(enumlabel),1,2)='''b' AND
(pg_catalog.quote_ident(typname)='e2' OR '"' || typname ||
'"'='e2') AND pg_catalog.pg_type_is_visible(t.oid) LIMIT 1000
I tried quickly fiddling with the substring() call to correct for the
changed offset somehow, but didn't succeed. Needs more analysis.
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
This doesn't work anymore:
create type e2 as enum ('foo', 'bar');
alter type e2 rename value 'b<TAB>
This now results in
alter type e2 rename value 'b'
Ugh. I'll take a look.
regards, tom lane
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
This doesn't work anymore:
create type e2 as enum ('foo', 'bar');
alter type e2 rename value 'b<TAB>
This now results in
alter type e2 rename value 'b'
The main issue here is that Query_for_list_of_enum_values[_with_schema]
is designed to match against a pre-quoted list of enum values,
which was appropriate when it was written because we hadn't configured
Readline to do anything special with quotes. Now that we have, the
string that is being supplied to match against lacks the leading quote,
so that we need to remove the quote_literal() calls from those queries.
A secondary problem is that if we fail to identify any match, Readline
nonetheless decides to append a trailing quote. That's why you get the
added quote above, and it still happens with the query fix. I'm not
sure why Readline thinks it should do that. I worked around it in the
attached draft patch by setting rl_completion_suppress_quote = 1 in our
failure-to-match case, but that feels like using a big hammer rather
than a proper solution.
I'm not totally satisfied with this patch for a couple of reasons:
1. It'll allow the user to enter a non-quoted enum value,
for example alter type e2 rename value b<TAB>
produces
alter type e2 rename value bar
It's not clear to me that there's any way around that, though.
I tried returning pre-quoted values as we did before (ie,
changing only the WHERE clauses in the queries) but then
Readline fails to match anything. We do have code to force
quoting of actual filenames, but I think that's dependent on
going through rl_filename_completion_function(), which of course
we can't do here.
2. It doesn't seem like there's any nice way to deal with enum
values that contain single quotes (which need to be doubled).
Admittedly the use-case for that is probably epsilon, but
it annoys me that it doesn't work.
In the end, it seems like the value of this specific completion
rule is not large enough to justify doing a ton of work to
eliminate #1 or #2. So I propose doing the attached and calling
it good. Maybe we could add a test case.
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.
regards, tom lane
Attachments:
enum-completion-1.patchtext/x-diff; charset=us-ascii; name=enum-completion-1.patchDownload+8-4
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