microsoft / go-sqlcmd

The new sqlcmd, CLI for SQL Server and Azure SQL (winget install sqlcmd / sqlcmd create mssql / sqlcmd open ads)
https://learn.microsoft.com/sql/tools/sqlcmd/go-sqlcmd-utility
MIT License
339 stars 60 forks source link

Feature request: support ADO.NET connection strings #478

Open ericsampson opened 10 months ago

ericsampson commented 10 months ago

It would be very useful if there was a way to pass in an ADO.NET-style connection string instead of separate server/user/pw/etc params.

This is already supported by go-mssqldb, so I'm hoping that the amount of work required to add this feature wouldn't be excessive :)

If this sounds like a good idea, and we can get agreement on how the UX should look (a new flag --connection-string/-S perhaps), then I can probably work on creating a PR for this feature :)

Thanks, Eric

shueybubbles commented 10 months ago

thx for opening an issue! I agree that having switches per parameter isn't a sustainable model. It's a problem across the entire SQL toolset, from powershell cmdlets to bcp to SSMS.

How about following the same pattern as sqlcmd -v where we have a new flag that provides an arbitrary named parameter to append to the connection string?

sqlcmd -p applicationclientid=myid -p "clientcertpath=c:\certfolder\file.cer"
shueybubbles commented 10 months ago

-p isn't a good candidate because it'll be used for print statistics but any available letter would work.

dlevy-msft commented 10 months ago

It would be really interesting to be able to pass a JSON string or even the path or URL to a JSON file kind of like a .net app.config file.

shueybubbles commented 10 months ago

the context file sqlcmd creates for "modern" callers is that JSON file. sqlcmd in back-compat mode will use the default context if SQLCMDSERVER env var is not set and you don't pass -S on the command line. The new flag would be useful in modern mode to configure the context too.

ericsampson commented 10 months ago

It's a problem across the entire SQL toolset, from powershell cmdlets to bcp to SSMS.

The interesting thing is that the powershell invoke-sqlcmd cmdlet already supports a -ConnectionString argument just like I'm suggesting... 😄 https://learn.microsoft.com/en-us/powershell/module/sqlserver/invoke-sqlcmd?view=sqlserver-ps#-connectionstring

ericsampson commented 10 months ago

How about following the same pattern as sqlcmd -v where we have a new flag that provides an arbitrary named parameter to append to the connection string?

That could potentially work, assuming that it would be fine to pass only the entire ado.net style connection string without having to specify anything else... How would you propose handling conflicts between explicit params and the ones contained in this "additional KV string" ?

Having said that, I was hoping for a simple solution that would let the user pass in an ado.net connection string and then the sqlcmd code could hand it off unparsed to go-mssql, rather than something like the above that seems significantly more complicated in terms of having to map the keys into connection params, check for invalid entries, etc etc. Unless I'm overthinking things, and it would be simple to take the results of StringToStringVarP and splat them into the required connection struct fields ?

shueybubbles commented 10 months ago

I think both types of parameters could work: 1, a --connection-string that can't be passed in conjunction with any other connection related parameters and is passed "as is" to the driver, and 2) a --connection-parameter param=value which can have multiple entries. The params from the latter would be applied last so they override any values that came from environment variables or other flags.

Supporting both approaches broadens the audience. There are a lot of existing legacy sqlcmd users who probably just need to pass one or two new values to the connection string and they could modify their scripts by appending --connection-parameter values to it.

ericsampson commented 10 months ago

That sounds good to me David! Do you have suggested short-form flag names for both of these? Finding good somewhat-meaningful short names is a challenge given the ones that are already taken lol. I think that -S for --connection-string seems reasonable, if that sounds ok to you.

I'll probably take a swipe at a PR for the "whole connection string" feature, and then the other one can be a followup. I'm not sure if I have time for both, and my personal usecase is for the former :D

Cheers

shueybubbles commented 10 months ago

This is the same as #108

ericsampson commented 10 months ago

@shueybubbles I have the feature nearly done locally :)

I'm running into one issue regarding a unit test, what's a good way for me to ask someone for a little guidance?

I could open up a draft PR with the current work, and add a comment regarding the issue I'm having.

shueybubbles commented 10 months ago

a draft PR is probably the best option