Getting PSQL in Windows to support TAB/Autocomplete via modified readline...

Kirk Wolak <wolakk@gmail.com> writes:

We have our team of Windows developers, leveraging PSQL. But honestly,
it feels crippled after using PSQL in Linux for any length of time. Losing
auto-complete sucks (multi-line query recall/editing is lost as well).

In researching this problem, it appears that the decision was made like
17yrs ago, when windows did not have a realistic "terminal" type
interface. Assuming we target Windows 8.1 or higher, I believe this goes
away.

It'd certainly be nice if we could use Readline on Windows. I do not
think we want to buy into maintaining our own fork of Readline, if that's
what you're trying to suggest. If it "just works" now, that'd be great.
Otherwise, maybe you should be speaking to the Readline maintainers
about what changes are needed in it?

regards, tom lane

On Mon, Nov 21, 2022 at 4:58 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Kirk Wolak <wolakk@gmail.com> writes:

We have our team of Windows developers, leveraging PSQL. But honestly,
it feels crippled after using PSQL in Linux for any length of time. Losing
auto-complete sucks (multi-line query recall/editing is lost as well).

In researching this problem, it appears that the decision was made like
17yrs ago, when windows did not have a realistic "terminal" type
interface. Assuming we target Windows 8.1 or higher, I believe this goes
away.

It'd certainly be nice if we could use Readline on Windows. I do not
think we want to buy into maintaining our own fork of Readline, if that's
what you're trying to suggest. If it "just works" now, that'd be great.
Otherwise, maybe you should be speaking to the Readline maintainers
about what changes are needed in it?

FWIW, I've been using https://github.com/arangodb/linenoise-ng for
Linux and Windows,
but that's C++ and no longer supported (but worked well enough for me).

The main https://github.com/antirez/linenoise itself does not build
as-is on Windows still, I believe. --DD

On Mon, Nov 21, 2022 at 10:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

It'd certainly be nice if we could use Readline on Windows. I do not
think we want to buy into maintaining our own fork of Readline, if that's
what you're trying to suggest. If it "just works" now, that'd be great.

Okay, we should know shortly if it works and to what extent. We will
take the approach that if we don't need to maintain a fork we are fine.
[The latter could make it work only for English, not sure]

FYI,

On Mon, Nov 21, 2022 at 11:01 AM Dominique Devienne <ddevienne@gmail.com>
wrote:

FWIW, I've been using https://github.com/arangodb/linenoise-ng for Linux

and Windows,

I've look at linenoise and without tab/autocomplete it's not worth the
effort, IMO.
But I appreciate the pointer...

regards

On Mon, Nov 21, 2022 at 6:12 PM Kirk Wolak <wolakk@gmail.com> wrote:

On Mon, Nov 21, 2022 at 11:01 AM Dominique Devienne <ddevienne@gmail.com> wrote:

FWIW, I've been using https://github.com/arangodb/linenoise-ng for Linux and Windows,

I've look at linenoise and without tab/autocomplete it's not worth the effort, IMO.

Not sure to understand... Linenoise* has completion hooks.
Completion is something done in the app, not readline/linenoise.
Only the app has the context to know what to complete the text with.
But perhaps I'm missing something? --DD

https://github.com/arangodb/linenoise-ng/blob/master/include/linenoise.h

On Tue, Nov 22, 2022 at 4:25 AM Kirk Wolak <wolakk@gmail.com> wrote:

In researching this problem, it appears that the decision was made like 17yrs ago, when windows did not have a realistic "terminal" type interface. Assuming we target Windows 8.1 or higher, I believe this goes away.

FWIW PostgreSQL 16 will require Windows 10+. Not a Windows user
myself, but I read somewhere that Windows 8.1 has already started
showing full screen warnings at startup that security patches etc end
in January, while PostgreSQL 16 (the soonest release that could
include your changes) is expected late in the year.

What we are looking for is a simple bullet point list of what would make such a contribution acceptable. And how far should we get through that list on our own, before getting some help, especially from the build teams? [Our goal would be an NEW Config type flag: READLINE_FOR_WINDOWS (you guys name it, and control the default setting)]

Some thoughts:

Re configuration flags: don't waste time with the old perl-based build
system. The configuration should be done only with the new meson
build system (soon to be the only way to build on Windows).

I didn't quite understand if you were saying that readline itself
needs patches for this (I gather from earlier threads about this that
there were some problems with dll symbol export stuff, so maybe that's
it?). In passing, I noticed that there is also a Windows port of
editline AKA libedit, the BSD-licensed alternative to readline. It
has a compatible API and PostgreSQL can use that too. I have no idea
which is easier, more supported, etc on Windows.

It's OK to post a work-in-progress patch to pgsql-hackers, even if it
doesn't work right yet. With any luck, people will show up to help
with problems. I am 100% sure that our Windows user community would
love this feature. It would be good if the tests in
src/bin/psql/t/010_tab_completion.pl pass on Windows, but if that's
hard, don't let that stop you sharing a patch.

On Mon, Nov 21, 2022 at 1:10 PM Dominique Devienne <ddevienne@gmail.com>
wrote:

On Mon, Nov 21, 2022 at 6:12 PM Kirk Wolak <wolakk@gmail.com> wrote:

On Mon, Nov 21, 2022 at 11:01 AM Dominique Devienne <ddevienne@gmail.com>

wrote:

FWIW, I've been using https://github.com/arangodb/linenoise-ng for

Linux and Windows,

I've look at linenoise and without tab/autocomplete it's not worth the

effort, IMO.

Not sure to understand... Linenoise* has completion hooks.
Completion is something done in the app, not readline/linenoise.
Only the app has the context to know what to complete the text with.
But perhaps I'm missing something? --DD

https://github.com/arangodb/linenoise-ng/blob/master/include/linenoise.h

I missed that. There is a hook for it. Thank you. Apologies.

I just got the first pass of readline working in windows with psql!!
it's "rough" right now, I will have to work through a bunch of
assumptions...
but it's WORKING! (at least in utf8)... Tons of testing, and some of this
code has not
been compiled for Windows in 17yrs. I might need to find one of the
geniuses who
understands the inner workings of psqlscanslash...

To avoid pasting a screenshot, here is a link to the auto-complete prompt:
https://app.screencast.com/RBwKbZeXFdBnO

On Tue, Nov 22, 2022 at 5:51 PM Thomas Munro <thomas.munro@gmail.com> wrote:

On Tue, Nov 22, 2022 at 4:25 AM Kirk Wolak <wolakk@gmail.com> wrote:

In researching this problem, it appears that the decision was made

like 17yrs ago, when windows did not have a realistic "terminal" type
interface. Assuming we target Windows 8.1 or higher, I believe this goes
away.

FWIW PostgreSQL 16 will require Windows 10+. Not a Windows user
...
Some thoughts:

Re configuration flags: don't waste time with the old perl-based build
system. The configuration should be done only with the new meson
build system (soon to be the only way to build on Windows).

Perl? I am building/running in VS2022 community edition.

https://app.screencast.com/RBwKbZeXFdBnO (screenshot) just working today (5
long days!)

I didn't quite understand if you were saying that readline itself
needs patches for this (I gather from earlier threads about this that
there were some problems with dll symbol export stuff, so maybe that's
it?). ...

yeah, readline does not build well in windows without some "tweaks", I am
working with someone.
They are reaching back to see if we can get the updates pushed back into
readline, as Tom
requested we not require supporting our own branch. Currently it's a
single patch file,
plus some "glue"... (putting the terminal in CHAR mode, and binary mode vs
line mode, etc)

It's OK to post a work-in-progress patch to pgsql-hackers, even if it
doesn't work right yet. With any luck, people will show up to help
with problems. I am 100% sure that our Windows user community would
love this feature. It would be good if the tests in
src/bin/psql/t/010_tab_completion.pl pass on Windows, but if that's
hard, don't let that stop you sharing a patch.

Thomas, thanks for that! So new to this, I didn't realize... That's a
great idea.
Honestly not sure how to even run it?

I probably should learn how to build with your build system.
And how to submit a patch. Let me get it working better (I don't think it
does multi-line yet).

Thanks for the support, it's encouraging... especially when I know there's
an 80% chance that
this may fail to get accepted for any number of reasons.

Am Tue, Nov 22, 2022 at 11:59:59PM -0500 schrieb Kirk Wolak:

It's OK to post a work-in-progress patch to pgsql-hackers, even if it
doesn't work right yet. With any luck, people will show up to help
with problems. I am 100% sure that our Windows user community would
love this feature. It would be good if the tests in
src/bin/psql/t/010_tab_completion.pl pass on Windows, but if that's
hard, don't let that stop you sharing a patch.

Thomas, thanks for that! So new to this, I didn't realize... That's a
great idea.
Honestly not sure how to even run it?

Thanks for the support, it's encouraging... especially when I know there's
an 80% chance that
this may fail to get accepted for any number of reasons.

I don't think that estimate needs to be that pessimistic.

Thanks for the effort to bring tab completion to psql on windows.

Karsten
--
GPG 40BE 5B0E C98E 1713 AFA6 5BC0 3BEA AC80 7D4F C89B

On Wed, 2022-11-23 at 12:56 +0100, Karsten Hilbert wrote:

Am Tue, Nov 22, 2022 at 11:59:59PM -0500 schrieb Kirk Wolak:

It's OK to post a work-in-progress patch to pgsql-hackers, even if it
doesn't work right yet.  With any luck, people will show up to help
with problems.  I am 100% sure that our Windows user community would
love this feature.  It would be good if the tests in
src/bin/psql/t/010_tab_completion.pl pass on Windows, but if that's
hard, don't let that stop you sharing a patch.

Thomas,  thanks for that!  So new to this, I didn't realize...  That's a
great idea.
Honestly not sure how to even run it?

Thanks for the support, it's encouraging...  especially when I know there's
an 80% chance that
this may fail to get accepted for any number of reasons.

I don't think that estimate needs to be that pessimistic.

Thanks for the effort to bring tab completion to psql on windows.

Yes, that would be a great achievement!

Yours,
Laurenz Albe

On Wed, Nov 23, 2022 at 6:56 AM Karsten Hilbert <Karsten.Hilbert@gmx.net>
wrote:

Am Tue, Nov 22, 2022 at 11:59:59PM -0500 schrieb Kirk Wolak:

It's OK to post a work-in-progress patch to pgsql-hackers, even if it
src/bin/psql/t/010_tab_completion.pl pass on Windows, but if that's
hard, don't let that stop you sharing a patch.

Okay, hours wasted. IO::Pty does not exist for win32 (yet)...
I ran the tests "manually" (OMG, I know why we automate this).

Anyways,* ALL tests passed EXCEPT filename completion* which crashes. LOL
And a spurious extra space output on "SELECT * FR\t" -> "SELECT * FR<space>"
when there should be none. Not in all situations, seems like ONLY when
suggestions
don't exist. I will attack this next.

And -n works! (Turning off readline)

Thanks for the support, it's encouraging... especially when I know

there's

an 80% chance that
this may fail to get accepted for any number of reasons.

I don't think that estimate needs to be that pessimistic.

Karsten, you haven't seen the test results and the "walled garden" of
limitations I've created. :-)

I thought that was optimistic for a first-timer :-)
- Limited to UTF8 Support. (for now) [we use ACP calls, normally]
- Forces Binary Mode. Resetting terminal settings going in/out (and in
Mainloop).
- Chasing down memory leaks
- Not tested with redirection/piping or -i stuff yet.
- Not tested with "Difficult" Languages (Russian, CH, JA, KO, etc)

Any one of those could become a deal killer.

Regards Kirk

hi,

On 2022-11-21 10:58:06 -0500, Tom Lane wrote:

Kirk Wolak <wolakk@gmail.com> writes:

We have our team of Windows developers, leveraging PSQL. But honestly,
it feels crippled after using PSQL in Linux for any length of time. Losing
auto-complete sucks (multi-line query recall/editing is lost as well).

In researching this problem, it appears that the decision was made like
17yrs ago, when windows did not have a realistic "terminal" type
interface. Assuming we target Windows 8.1 or higher, I believe this goes
away.

It'd certainly be nice if we could use Readline on Windows.

FWIW, when building postgres with meson and mingw, you end up with a working
readline today (provided readline is installed). That ended up working mostly
accidentally. See here for a few more details:
/messages/by-id/20220928022724.erzuk5v4ai4b53do@awork3.anarazel.de

I think there might still be something weird with ctrl-c, but that's not
really related to readline.

Readline doesn't currently work when building msvc for two reasons:

1) Readline doesn't provide an import library. Mingw implements a workaround
for that by basically generating one at link time, but msvc doesn't.

An import library can be fairly easily be generated. Or one could propose a
patch to upstream readline to add support for generating readline.

2) The last time I checked, msvc couldn't preprocess tab-complete.c with
USE_READLINE defined, due to running into some preprocessor limits. We can
probably fix that on our end somehow.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

On 2022-11-21 10:58:06 -0500, Tom Lane wrote:

It'd certainly be nice if we could use Readline on Windows.

2) The last time I checked, msvc couldn't preprocess tab-complete.c with
USE_READLINE defined, due to running into some preprocessor limits. We can
probably fix that on our end somehow.

Huh ... do you recall the details? Large as tab-complete is, it's
far smaller than gram.y:

$ ll src/bin/psql/tab-complete.*
-rw-rw-r--. 1 postgres postgres 227042 Nov 19 19:37 src/bin/psql/tab-complete.c
-rw-rw-r--. 1 postgres postgres 347 Apr 11 2022 src/bin/psql/tab-complete.h
-rw-rw-r--. 1 postgres postgres 838792 Nov 23 10:02 src/bin/psql/tab-complete.o
$ size src/bin/psql/tab-complete.o
text data bss dec hex filename
148550 0 128 148678 244c6 src/bin/psql/tab-complete.o
$ ll src/backend/parser/gram.*
-rw-rw-r--. 1 postgres postgres 2506806 Nov 21 10:35 src/backend/parser/gram.c
-rw-rw-r--. 1 postgres postgres 12861 Nov 21 10:35 src/backend/parser/gram.h
-rw-rw-r--. 1 postgres postgres 2183184 Nov 23 10:02 src/backend/parser/gram.o
-rw-rw-r--. 1 postgres postgres 476700 Nov 21 10:34 src/backend/parser/gram.y
$ size src/backend/parser/gram.o
text data bss dec hex filename
620127 0 0 620127 9765f src/backend/parser/gram.o

So I'm having a hard time guessing why we'd hit compiler limits
in tab-complete if we can build the rest of the system.

regards, tom lane

Hi,

On 2022-11-23 18:11:22 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2022-11-21 10:58:06 -0500, Tom Lane wrote:

It'd certainly be nice if we could use Readline on Windows.

2) The last time I checked, msvc couldn't preprocess tab-complete.c with
USE_READLINE defined, due to running into some preprocessor limits. We can
probably fix that on our end somehow.

Huh ... do you recall the details? Large as tab-complete is, it's
far smaller than gram.y:

I think the problem was that msvc couldn't quite deal with VA_ARGS_NARGS. Or
something like that.

<tinker, curse, tinker>

Ah, yes:
../src/bin/psql/tab-complete.c(4064): fatal error C1026: parser stack overflow, program too complex

We discussed something around VA_ARGS_NARGS not working on windows:
/messages/by-id/f450fc57-a147-19d0-e50c-33571c52cc13@postgrespro.ru
But now I don't immediately see the above error as being related.

If I remove all the completions after
/* Complete INSERT INTO with table names */
else if (TailMatches("INSERT", "INTO"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_updatables);

it builds with msvc as well. And when using a readline sourced from vcpkg, it
even links. Of course only a few of the completions work, given that I
truncated a whole lot of them away...

So It might just be that we need to split up that very long "else if" chain in
psql_completion().

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

On 2022-11-23 18:11:22 -0500, Tom Lane wrote:

Huh ... do you recall the details? Large as tab-complete is, it's
far smaller than gram.y:

So It might just be that we need to split up that very long "else if" chain in
psql_completion().

Could be, yeah. Related point: I've wondered for awhile how long that
chain can get before we start noticing performance issues on slower
machines. Even when the standard is just human reaction time, there's
a limit to how many cycles you can throw away. So I'd like to see it
refactored somehow to be a bit less stupid --- but I have a nagging
feeling that we'd end up building some special-purpose program generator,
which is something I've not wanted to do. OTOH, in the best of all
possible worlds such a tool might make it easier to add tab
completions?

(In the past I've fantasized about auto-generating tab completion
logic from the backend grammar, but I fear it's just fantasy.
The backend grammar isn't factored right, and too many of its names
don't have clear traceability to what-kind-of-object-is-that.)

regards, tom lane

On Wed, Nov 23, 2022 at 6:11 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

On 2022-11-21 10:58:06 -0500, Tom Lane wrote:

It'd certainly be nice if we could use Readline on Windows.

2) The last time I checked, msvc couldn't preprocess tab-complete.c with
USE_READLINE defined, due to running into some preprocessor limits.

We can

probably fix that on our end somehow.

Huh ... do you recall the details? Large as tab-complete is, it's
far smaller than gram.y:
...

So I'm having a hard time guessing why we'd hit compiler limits

in tab-complete if we can build the rest of the system.

regards, tom lane

Oh, I hit them. In the tab-complete.c you have a SINGLE if/else that spans
over 2,000 lines.
And it blows up the STACK of the C Compiler (stack overflow). (It took 1
variable (if_continues)) and ENDING the if, and restarting it.

Also, the PREPROCESSOR you need the flag: "Use Standard Conforming
Processor" (or V_ARGS and NARGS breaks).
[ (/Zc:preprocessor) ]

of course SOMEONE should ask WHY use a non-conforming pre-processor??? It
cost me hours...

Currently I have a lot of it working, but I need to partner up with some of
the meson guys...
if it is already close to working, a couple of my tweaks could accelerate
it.

A Truly Happy Thanksgiving!

On Wed, Nov 23, 2022 at 7:41 PM Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2022-11-23 18:11:22 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2022-11-21 10:58:06 -0500, Tom Lane wrote:

It'd certainly be nice if we could use Readline on Windows.

So It might just be that we need to split up that very long "else if"
chain in
psql_completion().
Andres Freund

That's exactly what I did:
int if_continues = 0; // declared at the top

Circa Line 3,900
else
if_continues = 1; // break huge if
if (if_continues == 0)
NULL;
else ... // This else picks up here again

and you could replace the NULL with if_continues = 0; to allow breaking it
again in the future with the same variable.
I've never seen a single statement that long...

Regards Kirk

Hi,

On 2022-11-23 20:28:29 -0500, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2022-11-23 18:11:22 -0500, Tom Lane wrote:

Huh ... do you recall the details? Large as tab-complete is, it's
far smaller than gram.y:

So It might just be that we need to split up that very long "else if" chain in
psql_completion().

Could be, yeah.

It's not quite correct, but to test that I wrapped all the Matches("ALTER",
...) into a if (HeadMatches("ALTER")). That's enough to make the file compile
without any other changes.

The problem with that of course is that we then don't try the later
completions if there's no match for any of the Matches("ALTER", ...).

I think it'd be easier to deal with this if COMPLETE_WITH_* caused the the
containing function to return. Then the completions wouldn't need to be in one
huge if-else if. Leaving msvc aside, that also seems nice for efficiency.

Related point: I've wondered for awhile how long that
chain can get before we start noticing performance issues on slower
machines.

Same.

Even when the standard is just human reaction time, there's
a limit to how many cycles you can throw away. So I'd like to see it
refactored somehow to be a bit less stupid --- but I have a nagging
feeling that we'd end up building some special-purpose program generator,
which is something I've not wanted to do. OTOH, in the best of all
possible worlds such a tool might make it easier to add tab
completions?

(In the past I've fantasized about auto-generating tab completion
logic from the backend grammar, but I fear it's just fantasy.
The backend grammar isn't factored right, and too many of its names
don't have clear traceability to what-kind-of-object-is-that.)

I've thought about a grammar based approach as well, but as you say, it's not
obvious how to make that work well.

I wonder if we could model the completions as something roughly akin to a
DFA. We don't even need to generate the real state machine at compile time,
it'd be fine to do it at psql startup.

But even just representing the current top-level conditions in an array, and
then having a few obvious optimizations when matching the input to the array,
should make it easy to beat the current approach. And it'd result in a much
smaller amount of .text.

There's a small number of else ifs that aren't just combinations of *Matches*
conditions, e.g. stuff like !ends_with(), but those could be dealt via
optional callbacks.

I'm, very roughly, thinking of something like:

compl_match_rule match_rules[] = {
{.match = Matches("CREATE"), .compl_func = create_command_generator},
{.match = TailMatches("CREATE", "OR", "REPLACE"), .compl = {"FUNCTION", "..."}},
...
{.match = MatchAnd(
HeadMatches("ALTER", "PUBLICATION", MatchAny),
TailMatches("WHERE")
),
.compl = {")"}},
...
}

where of course Matches() etc wouldn't directly generate code, but evaluate to
a literal struct with const char* members for the different options etc.

I think this should make it easier to optimize evaluation. We e.g. could
e.g. require that the Matches() rules are sorted, allowing to find the
appropriate Matches() / HeadMatches() starting with the word we're trying to
complete.

Greetings,

Andres Freund

Hi,

On 2022-11-23 20:55:18 -0500, Kirk Wolak wrote:

Currently I have a lot of it working, but I need to partner up with some of
the meson guys...
if it is already close to working, a couple of my tweaks could accelerate
it.

I just successfully compiled postgres with meson and readline, using msvc,
only tweaking psql_completion() contents.

The readline I used for building with msvc is from
vcpkg.exe install --triplet x64-windows readline-win32 pkgconf

I also installed icu zlib zstd libxml2 libxslt tcl that way, but shouldn't
matter here.

vcpkg is installed in c:/dev/vcpkg

Other steps:
# let meson know where to get pkgconf from, could also add it to PATH
$ENV:PKG_CONFIG="c:/dev/vcpkg/installed/x64-windows/tools/pkgconf/pkgconf.exe"

# specify location of pkgconfig files, so meson can get compilation flags
meson setup --buildtype debug -Dpkg_config_path=c:/dev/vcpkg/installed/x64-windows/debug/lib/pkgconfig ...

# when running tests, the readline dll needs to be somewhere in PATH
$ENV:PATH="$ENV:PATH;c:/dev/vcpkg/installed/x64-windows/debug/bin"

If you want to build an optimized postgres, you'd need to change the
pkg_config_path and PATH so it doesn't include debug/ (this is because of
needing to link to the same CRT).

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:

I think it'd be easier to deal with this if COMPLETE_WITH_* caused the the
containing function to return. Then the completions wouldn't need to be in one
huge if-else if. Leaving msvc aside, that also seems nice for efficiency.

Yeah, that could be an easy quick-fix. We'd need to refactor so that
the cleanup housekeeping at the bottom of psql_completion() was in a
wrapper function, but perhaps that wouldn't be too messy.

regards, tom lane