Horton

Thread

Commits

  1. Remove configure's check for rl_completion_append_character.

  2. Improve psql tab-completion tests.

  1. psql tab completion versus Debian's libedit

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-02-01T21:30:11Z

    I chased down the failure that kittiwake has been showing since
02b8048ba [1].  It's not hard to reproduce if you have an older
Debian release and you build --with-libedit-preferred.
Manual experimentation shows that when these versions of libedit
complete a string containing double quotes, they insist on
backslashing all the double quotes :-(.  That is, if you have
a table "foobar" and you type

select * from "foo<TAB>

what you'll get is

select * from \"foobar\"

Digging into the source code, I find that libedit versions prior
to 3.1-20210522 are unshakably convinced that anything you are
completing should be escaped per shell quoting rules :-(.  AFAICT
there is no way to turn that off without replacing the *entire*
tab completion infrastructure.

3.1-20210522 changed this to the extent of disabling quoting
if the application supplies a rl_attempted_completion_function,
as we do.  (Fine for us, sucks for anybody who did want shell
quoting.)

I'm not too sure about the relationship of Debian's version of
libedit to anyone else's.  Apple's version, for one, lacks this
bug.  But it does appear that you don't want to use libedit on
Debian unless you have a very late-model release.

We can paper over the regression failure by making the test
cases allow backslashes, but I wonder if we need something
in the documentation discouraging people from choosing these
versions of libedit over readline.  They're pretty broken for
completions involving double-quoted names even before 02b8048ba.

			regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=kittiwake&dt=2022-01-31%2016%3A24%3A48

  2. Re: psql tab completion versus Debian's libedit

    Andres Freund <andres@anarazel.de> — 2022-02-03T02:10:29Z

    Hi,

On 2022-02-01 16:30:11 -0500, Tom Lane wrote:
> I chased down the failure that kittiwake has been showing since
> 02b8048ba [1].

I just rebased my meson branch across the commit d33a81203e9. And on freebsd
the meson based build failed in the expanded tests, while autoconf succeeded.

The failure is:

not ok 22 - complete schema-qualified name
#   Failed test 'complete schema-qualified name'
#   at /tmp/cirrus-ci-build/src/bin/psql/t/010_tab_completion.pl line 236.
# Actual output was "tab "
# Did not match "(?^:tab1 )"


I think this is caused by the feature flag detection being broken in the meson
branch - unrelated to your commit - ending up with falsely believing that none
of the rl_* variables exist (below for more on that aspect).

Do we care that the tests would fail when using a readline without any of the
rl_* variables? I don't know if those even exist.


The reason for meson not detecting the variables is either an "andres" or
freebsd / readline issue. The tests fail with:

/usr/local/include/readline/rltypedefs.h:71:36: error: unknown type name 'FILE'
typedef int rl_getc_func_t PARAMS((FILE *));
                                   ^
apparently the readline header on freebsd somehow has a dependency on stdio.h
being included.

Looks like it's easy enough to work around. My local copy of readline.h (8.1
on debian sid) has an explicit stdio.h include, but it looks like that's a
debian addition...

Greetings,

Andres Freund

  3. Re: psql tab completion versus Debian's libedit

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-02-03T03:28:31Z

    Andres Freund <andres@anarazel.de> writes:
> I think this is caused by the feature flag detection being broken in the meson
> branch - unrelated to your commit - ending up with falsely believing that none
> of the rl_* variables exist (below for more on that aspect).
> Do we care that the tests would fail when using a readline without any of the
> rl_* variables? I don't know if those even exist.

Hm, interesting.  I reproduced this by #undef'ing
HAVE_RL_COMPLETION_APPEND_CHARACTER in pg_config.h.  It's not too
surprising, because without that, we have no way to prevent readline
from appending a space to a completion.  In the test at hand, that
causes two failures:

1. After we do "pub<TAB>", we get "public. " not just "public.";
and now, that is no longer part of the word-to-be-completed.
That breaks the test because FROM is no longer the previous word
but the one before that, so we won't try to do table completion.

2. After we do "tab<TAB>", we normally get "tab1 ", but since no
completion is attempted, we should get just "tab".  But we
get "tab " because the dummy-completion code at the bottom
of psql_completion fails to suppress adding a space.  In general,
*any* failed completion would nonetheless append a space.

Each of these is sufficiently bad to prompt user complaints,
I think, but we've seen none.  And I observe that every buildfarm
animal reports having rl_completion_append_character.

I conclude that there are no extant versions of readline/libedit
that don't have rl_completion_append_character, so we could
drop that configure test and save a cycle or two.

			regards, tom lane

  4. Re: psql tab completion versus Debian's libedit

    Andres Freund <andres@anarazel.de> — 2022-02-03T03:33:43Z

    On 2022-02-02 22:28:31 -0500, Tom Lane wrote:
> I conclude that there are no extant versions of readline/libedit
> that don't have rl_completion_append_character, so we could
> drop that configure test and save a cycle or two.

Sounds good to me!

  5. Re: psql tab completion versus Debian's libedit

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-02-03T03:40:52Z

    Andres Freund <andres@anarazel.de> writes:
> On 2022-02-02 22:28:31 -0500, Tom Lane wrote:
>> I conclude that there are no extant versions of readline/libedit
>> that don't have rl_completion_append_character, so we could
>> drop that configure test and save a cycle or two.

> Sounds good to me!

On it now.

			regards, tom lane