search

tpope / vim-dadbod

dadbod.vim: Modern database interface for Vim
https://www.vim.org/scripts/script.php?script_id=5665
3.75k stars 132 forks source link

sqlserver support on windows not working since switch to jobs #128

Closed gbarta closed 1 year ago

gbarta commented 1 year ago

Reproduced with Windows 10 and Vim 9.0.1425 x64. It works ok with Neovim (0.8.3)

I was trying to open an interactive window with :DB sqlserver://machine/database, but it just does nothing.

Bisect shows 70f86045 as the first bad commit, prior to that it would open the interactive window.

The synthetic s:systemlist(...) function used to test the connection doesn't return anything and treats the command as a failure for sqlcmd, but systemlist(...) and system(...) work fine. I simplified it to returning the help text with 'sqlcmd -?' with the same effect:

:Verbose echo systemlist('sqlcmd -?')
['Microsoft (R) SQL Server Command Line Tool', 'Version 15.0.1300.359 NT ... lots of lines of cmd help ..

:Verbose echo <SNR>227_systemlist('sqlcmd -?','')
[[], 1]

The s:systemlist(...) function works for any other command I have tried, just not sqlcmd.

Interestingly, procmon shows that the sqlcmd process does in fact start. It's puzzling, I wonder if it is something special that sqlcmd is doing with the console, and it just doesn't like to be run from job_start().

tpope commented 1 year ago

127 was opened 2 days ago and strongly implies the underlying plumbing works correctly. You're not using s:systemlist() like Dadbod itself does; try with ['sqlcmd', '-?'], ''.

gbarta commented 1 year ago

Sorry, I did originally use the exact command list constructed by Dadbod, but when simplifying it and comparing it with systemlist() it was changed. I get the same result with the list version:

:Verbose echo <SNR>274_systemlist(['sqlcmd','-?'],'')
[[], 1]

127 refers to running sql server in a docker container. It isn't definitive, but most likely this means using the linux version of sql server, which is different from the Windows version.

With some more poking around, I found that the behaviour is fixed if I remove the 'in_io': 'null' option from the job_start options at https://github.com/tpope/vim-dadbod/blob/a09e40664e9cd30cd2b3f8866b796598302070f6/autoload/db.vim#L201

This is suggestive that the problem is that the windows sqlcmd doesn't like being disconnected from stdin, even if it is not being used.

I haven't noticed any ill effects from removing this line, and from reading the vim docs I am not sure that it is required. It can be used to prevent creating a channel, but I think this is only in combination with the same value for out_io and err_io as well.

tpope commented 1 year ago

Leaving stdin open means that the channel is writable, which means that the running process would block when trying to read from stdin. This is not a fix I am willing to blindly throw at a poorly understood problem. Things you could try instead:

tpope commented 1 year ago

Also, it would be instructive to test if the same problem occurs on Neovim.

gbarta commented 1 year ago

It works OK in Neovim.

Your second option seems to also work, with the following added after job_start in addition to removing 'in_io': 'null':

    if !filereadable(a:in_file)
        call ch_close_in(state.job)
    endif

This seems similar to the code for nvim, which immediately calls chanclose(job, 'stdin')