Re: pgsql: Add basic TAP tests for psql's tab-completion logic.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Christoph Berg <myon(at)debian(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
Date: 2020-01-04 19:50:47
Message-ID: 22817.1578167447@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Peter Geoghegan <pg(at)bowt(dot)ie> writes:
> On Sat, Jan 4, 2020 at 10:19 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Looking closer, I see that your actual output included *both*
>> spaces and escape sequences between the table names, so it
>> needs to be more like the attached.

> This patch makes the tests pass. Thanks!

Ah, good. Pushed.

>> So now I'm thinking again that there must be something about
>> your colorized setup that triggers use of at least the first one.
>> But why didn't clearing the relevant environment variables
>> change anything?

> I don't know. It also failed with bash, which doesn't have any of that stuff.

I got another data point just now. I was experimenting with a proposed
fix for the current libedit problem [1], so I needed to build libedit
from source, and the path of least resistance was to do so on my RHEL6
workstation. I find that that conglomeration fails the current
010_tab_completion.pl tests, even with this patch, with symptoms like

not ok 2 - complete SEL<tab> to SELECT

# Failed test 'complete SEL<tab> to SELECT'
# at t/010_tab_completion.pl line 91.
# Actual output was "postgres=# SEL\a\r\e[15GECT "

and similarly in a couple other tests. I know where the bell (\a)
is coming from: there's a different logic bug in libedit that causes
it to do el_beep() even when there's a unique completion. But why the
unnecessary cursor repositioning? Looking closer, I realize that
libedit on this software stack is depending on

libtinfo.so.5 => /lib64/libtinfo.so.5 (0x00007fa39a5e2000)

which is from

$ rpm -qf /lib64/libtinfo.so.5
ncurses-libs-5.7-4.20090207.el6.x86_64

Seeing that you're also having issues with a stack involving
libtinfo.so.5, here's my theory: libtinfo version 5 is brain-dead
about whether it needs to issue cursor repositioning commands, and
tends to do so even when the cursor is in the right place already.
Version 6 fixed that, which is why we're not seeing these escape
sequences on any of the libedit-using buildfarm critters.

So we're going to have to make some decisions about what range of
libedit builds we want to cater for in the tab-completion tests.
Given that the plan is to considerably increase the coverage of
those tests, I'm hesitant to promise that we'll keep passing with
anything that isn't explicitly tested in the buildfarm, or by active
developers such as yourself. Maybe that's fine given that end users
probably don't run the TAP tests.

It would likely help if we could constrain the behavior to avoid
variations such as colorization, which is why I'm concerned that
we don't seem to have figured out how to turn that off in your
setup. I think that bears more investigation, even though we've
managed to work around it for the moment.

regards, tom lane

[1] http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=54510

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2020-01-04 19:58:58 Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
Previous Message Noah Misch 2020-01-04 19:32:26 pgsql: Skip memcpy(x, x) in qunique().

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-01-04 19:58:58 Re: pgsql: Add basic TAP tests for psql's tab-completion logic.
Previous Message Justin Pryzby 2020-01-04 19:34:16 Re: allow disabling indexscans without disabling bitmapscans