Thread: 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 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
farshould we get through that list on our own, before getting some help, especially from the build teams?  [Our goal
wouldbe 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.

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

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:
https://www.postgresql.org/message-id/20220928022724.erzuk5v4ai4b53do%40awork3.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:
https://postgr.es/m/f450fc57-a147-19d0-e50c-33571c52cc13%40postgrespro.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

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

Hi,

On 2022-11-28 17:22:19 -0500, Kirk Wolak wrote:
>   Thank you for this.  We have the build working with meson (w/o readline).
> I was not able to get "readline" to be recognized by meson (I've attached
> the log file)....
> It is "there" according to vcpkg.  (other projects have a .pc file this one
> is missing?)

The problem is likely indicated by this:
> env[PKG_CONFIG_PATH]: c:/vcpkg/installed/x64-windows/tools/pkgconf/pkgconf.exe

This needs to point to the directory containing the .pc files, not the
executable.

Greetings,

Andres Freund

Hi,

On 2022-12-08 09:52:52 -0500, Kirk Wolak wrote:
> On Wed, Dec 7, 2022 at 2:18 PM Andres Freund <andres@anarazel.de> wrote:
> 
> > Hi,
> >
> > On 2022-11-28 17:22:19 -0500, Kirk Wolak wrote:
> > >   Thank you for this.  We have the build working with meson (w/o
> > readline).
> > > I was not able to get "readline" to be recognized by meson (I've attached
> > > the log file)....
> > > It is "there" according to vcpkg.  (other projects have a .pc file this
> > one
> > > is missing?)
> >
> > The problem is likely indicated by this:
> > > env[PKG_CONFIG_PATH]:
> > c:/vcpkg/installed/x64-windows/tools/pkgconf/pkgconf.exe
> >
> > This needs to point to the directory containing the .pc files, not the
> > executable.

> Thanks..
>   The above, plus I ended up having to create the readline.pc files.

They should be automatically created by vcpkg.

Greetings,

Andres Freund