Skip to content

SQL Server adapter: update parameters for latest version of sqlcmd #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
johnybx wants to merge 1 commit into from
Closed

SQL Server adapter: update parameters for latest version of sqlcmd #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions autoload/db/adapter/sqlserver.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,30 @@ function! s:server(url) abort
\ (has_key(a:url, 'port') ? ',' . a:url.port : '')
endfunction

function! s:get_value_from_url(url, param)
return get(a:url.params, a:param, get(a:url.params, toupper(a:param[0]) . a:param[1 : -1], '0'))
endfunction

function! s:boolean_param_flag(url, param, flag) abort
let value = get(a:url.params, a:param, get(a:url.params, toupper(a:param[0]) . a:param[1 : -1], '0'))
let value = s:get_value_from_url(a:url, a:param)
return value =~# '^[1tTyY]' ? [a:flag] : []
endfunction

function! s:value_param_flag(url, param, flag) abort
let value = s:get_value_from_url(a:url, a:param)
return value == '0' ? [] : [a:flag, value]
endfunction

function! db#adapter#sqlserver#interactive(url) abort
let url = db#url#parse(a:url)
" Legacy parameters left just for backward compatibility with older sqlcmd: secure and
" trustServerCertificate
Comment on lines +54 to +55
Copy link
Copy Markdown
Owner

@tpope tpope May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're not legacy, keep them as is, and fix your additions to match. See https://learn.microsoft.com/en-us/sql/connect/jdbc/setting-the-connection-properties?view=sql-server-ver16#properties.

Copy link
Copy Markdown
Contributor Author

@johnybx johnybx May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah now I understand the naming ( I thought that parameters were derived from sqlcmd --help ) - but at least for secure parameter which is supposed to be encrypt there was change

In version 11.2.0 and up, encrypt has been changed from boolean to string, allowing for TDS 8.0 support when the property is set to strict.

Do you want to keep secure a boolean parameter and encrypt as string parameter or there should be just one parameter and type of value will be detected ?

Copy link
Copy Markdown
Owner

@tpope tpope May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since -N doesn't accept a string, and thus there's no way for us to do anything withencrypt=strict, I think we should just leave it as is for now?

Copy link
Copy Markdown
Contributor Author

@johnybx johnybx May 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is actually one of the reasons why I started this pull request - I am using latest sqlcmd:

❯ sqlcmd --version
sqlcmd: 1.0.0

and I cannot use -N as boolean parameter because it was changed

❯ sqlcmd -N
sqlcmd: error: --encrypt-connection: expected string value but got "EOL" (<EOL>)
❯ sqlcmd -N X
sqlcmd: error: --encrypt-connection must be one of "default","false","true","disable" but got "X"
❯ sqlcmd -N disable
unable to open tcp connection with host 'localhost:1433': dial tcp [::1]:1433: connect: connection refused

The idea was that I would add parameter while keeping old one so that the adapter is not broken for people with older version of sqlcmd.

Copy link
Copy Markdown
Contributor Author

@johnybx johnybx May 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rebased the PR and one more thing to note is that encrypt and trustServerCertificate are properties of connection according to specification but format, column-separator and trim-spaces are options of sqlcmd client.

Copy link
Copy Markdown
Owner

@tpope tpope May 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yeah, those have no business being in a URL. URLs specify how to connect to the database, not arbitrary command-line flags. There have been multiple requests of this nature lately (#129, #125) so I think we should add some sort of general interface for that.

As for -N, I'm pushing a fix to support -N on ?encrypt=1 and a string argument for anything else.

Copy link
Copy Markdown
Contributor Author

@johnybx johnybx May 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good, thanks.

return ['env', 'SQLCMDPASSWORD=' . get(url, 'password', ''), 'sqlcmd', '-S', s:server(url)] +
\ s:boolean_param_flag(url, 'encrypt', '-N') +
\ s:value_param_flag(url, 'encrypt', '-N') +
\ s:value_param_flag(url, 'column-separator', '-s') +
\ s:value_param_flag(url, 'format', '-F') +
\ s:boolean_param_flag(url, 'trim-spaces', '-W') +
\ s:boolean_param_flag(url, 'secure', '-N') +
\ s:boolean_param_flag(url, 'trustServerCertificate', '-C') +
\ (has_key(url, 'user') ? [] : ['-E']) +
\ db#url#as_argv(url, '', '', '', '-U ', '', '-d ')
Expand Down
7 changes: 5 additions & 2 deletions doc/dadbod.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,11 @@ SQL Server ~
sqlserver://[<user>[:<password>]@][<host>][:<port>]/[<database>]
sqlserver://[<host>[:<port>]][;user=<user>][;...]
<
Supported query parameters are `secure` and `trustServerCertificate`, which
correspond to connection properties of the same name.
Supported query parameters are `encrypt-connection`, `format`,
`column-separator`, `trim-spaces` and `trust-server-certificate`, which
correspond to connection properties of the same name (see `sqlcmd -?`). For
backward compatibility with older versions of sqlcmd the boolean query
parameters `secure` and `trustServerCertificate` can be used.

To set the `integratedSecurity` connection property and use a trusted
connection, omit the user and password.
Expand Down