go-mssqldb icon indicating copy to clipboard operation
go-mssqldb copied to clipboard

Possible regression or breaking change introduced?

Open marefr opened this issue 3 years ago • 5 comments

Describe the bug Possible regression or breaking change introduced in regards to handling of provided port number. Upgraded this library from v0.0.0-20190707035753-2be1aa521ff4 to v0.0.0-20200620013148-b91950f658ec, ref. Commits between those version: https://github.com/denisenkom/go-mssqldb/compare/2be1aa5...b91950f.

Different exception messages reported after upgrade: 
Unable to open tcp connection with host 'SERVERNAME:1433': dial tcp [..........2390:4701]:1433: connectex: No connection could be made because the target machine actively refused it.
Unable to open tcp connection with host 'SERVERINTSQL01:1433': dial tcp 10.10.1.17:1433: connect: connection refused
Unable to open tcp connection with host 'appabc12V0032:1433': dial tcp 10.10.1.17:1433: i/o timeout

To Reproduce Connect using a connecting string of the following format: server=<server\instance>;port=1433;database=<database>;user id=<user id>;password=<password> that uses server\instance and port 1433 to connect.

Expected behavior Connect successfully.

Additional context Confirmed to work if using port=0 (zero) instead of 1433 - is this a breaking change or a regression/unintentional change?

Original bug report: https://github.com/grafana/grafana/issues/27224

marefr avatar Sep 23 '20 16:09 marefr

Any feedback on this one? Thanks

marefr avatar Oct 14 '20 12:10 marefr

Sorry to wade in, but this is unreleased software so it's unrealistic to expect any guarantees against breaking changes. There isn't a single tag yet, let alone any releases. It's not really production-ready in this state.

rickb777 avatar Oct 21 '20 12:10 rickb777

@rickb777 sorry I didn't know this. Reading the readme it's not clear it's not production-ready, but I may have missed something? Regarding no releases/tags makes sense, but hard to know if you're not involved in the project.

Nevertheless, based on my description above I would expect the readme to be be updated to highlight that you cannot specify the port > 0 in certain cases. If you do that at least I know that the behavior has changed intentionally and can update on our side.

Thanks

marefr avatar Oct 28 '20 09:10 marefr

There is a large number of forks; my guess is that many users made forks so they could provide their own stability 'guarantees'. I shall probably have to do the same.

rickb777 avatar Oct 28 '20 09:10 rickb777

I was not able to reproduce this issue. It works with and without port, here are different connections strings I tried:

  • user id=sa;password=sa;server=192.168.1.32;port=50397;database=master
  • user id=sa;password=sa;server=192.168.1.32\sqlexpress;database=master
  • user id=sa;password=sa;server=192.168.1.32\sqlexpress;port=50397;database=master

I think you are specifying incorrect port number. When you have named instance it usually would not run on port 1433, instead it would run on some dynamically allocated port, like 50397 in my example.

When you specify port number, you are forcing sql client to use provided port instead of using instance resolution process.

Generally you should not provide instance name and port at the same time, because instance name would be ignored. I think sql client can be changed to log a warning message in this case.

denisenkom avatar Nov 03 '20 03:11 denisenkom