Skip to content
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.

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

database/sql: DB.Exec leaving SQL Server connections in an "Invalid cursor state" if they have multiple statements #28472

Closed
sponeil opened this issue Oct 29, 2018 · 10 comments

Comments

@sponeil
Copy link

@sponeil sponeil commented Oct 29, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go1.10.3

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

windows/amd64

What did you do?

Tried to execute SQL server statements with multiple INSERT/UPDATE calls. One simple justification for this is a query like this to avoid multiple round trips to UPDATE a single ROW, but then INSERT it if the row does not exist:

DECLARE @id INT, @a VARCHAR(255)
SELECT @id = ?, @val = ?
UPDATE [my_table] SET a = @a WHERE id = @id
IF @@ROWCOUNT = 0
INSERT INTO [my_table] (id, a) VALUES (@id, @a)

The query above does not return any result sets, but in ODBC it returns multiple "affected row" counts. If I try to Exec the statement above, it will work the first time (and return the first count), but it will leave the connection in an "Invalid cursor state", and any queries I try to run after that will fail.

I would've left a play.golang.org link, but it provides no way to test ODBC connections to SQL Servers.

What did you expect to see?

I expected the ODBC driver to NOT leave the connection in an invalid cursor state I can't recover from.

What did you see instead?

It left the the connection in an invalid cursor state.

IMO the ideal fix would be to add one more method to the sql.Result interface that can return an array of counts. It would be analogous to the new NextResultSet method for sql.Rows, but it would keep the client code simple. However, it would be acceptable to simply fix the ODBC driver so it stops leaving the connection in an invalid cursor state (even if it must drop the rest of the counts).

@kardianos kardianos self-assigned this Oct 30, 2018
@kardianos
Copy link
Contributor

@kardianos kardianos commented Oct 30, 2018

What do driver package are you using?

@kardianos kardianos changed the title sql.DB.Exec leaving SQL Server connections in an "Invalid cursor state" if they have multiple statements database/sql: DB.Exec leaving SQL Server connections in an "Invalid cursor state" if they have multiple statements Oct 30, 2018
@sponeil
Copy link
Author

@sponeil sponeil commented Oct 30, 2018

@kardianos
Copy link
Contributor

@kardianos kardianos commented Oct 30, 2018

I would like to support returning individual result counts and other meta informational or messages as they come off the wire. But that won't land until v2 of the package.

If you are accessing a SQL Server database, I would highly recommend accessing it through the native TDS interface using https://github.com/denisenkom/go-mssqldb . TDS supports multiple statements and even multiple results from a single query.

It sounds like this is an odbc client issue. @alexbrainman may be able to comment more.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Oct 30, 2018

If you are accessing a SQL Server database, I would highly recommend accessing it through the native TDS interface using https://github.com/denisenkom/go-mssqldb

I am second that. If github.com/denisenkom/go-mssqldb works for you, just use it.

It sounds like this is an odbc client issue.

Maybe it is. @sponeil I do not have much free time now days, but if you create an issue here https://github.com/alexbrainman/odbc/issues/new with complete steps for me to reproduce your issue, I will investigate it. Thank you

Alex

@sponeil
Copy link
Author

@sponeil sponeil commented Oct 30, 2018

Thanks. As Alex requested, I have created a new issue on his project with steps and a Go unit test file that reproduces the error.

While I'm not against using a native TDS library, I chose ODBC first because I need to connect to multiple types of database servers, and in cases like that I prefer to minimize dependencies on third-party libraries.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Oct 31, 2018

I chose ODBC first because I need to connect to multiple types of database servers

Fair enough. But database/sql package drivers are easy to replace. You could still use github.com/alexbrainman/odbc package to access non MS SQL databases. Does not mean you have use github.com/alexbrainman/odbc for MS SQL too.

Alex

@sponeil
Copy link
Author

@sponeil sponeil commented Nov 1, 2018

"Does not mean you have use github.com/alexbrainman/odbc for MS SQL too."

Yes, I know. Just because I would prefer to avoid to add (yet another) third-party library doesn't mean I can't.

As a side note, I tried downloading "github.com/denisenkom/go-mssqldb" to run a unit test with it, and simply importing it into my package (without even using it) causes an immediate "exit status 4294967295" any time I try running "go test" on that package. Comment out the import line, and the test runs just fine. IMO that is really odd given the fact that the library is pure Go. Can't seem to find any command-line args that will give me any more info than that exit status -1.

EDIT: Sorry, the exit code -1 was mine. I was unable to see what was happening because "go test" caches/hides all log messages I try to send to stdout (I really wish I could disable that), it was aborting in TestMain before the first test even started, and for some reason trying to set a breakpoint on any line in TestMain is causing the debugger to hang. Debugging tests seems to work fine if I set a breakpoint inside a test func, but not in TestMain.

EDIT2: FYI, once I got it running, Denise's mssqldb driver ran 12.5% slower than Alex's ODBC driver. Everything else was identical except for the fact that I had to replace "?" parameters with "@p" parameters in the query strings.

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Nov 2, 2018

Denise's mssqldb driver ran 12.5% slower than Alex's ODBC driver.

github.com/denisenkom/go-mssqldb package is pure Go. Written by people who had to guess how communication between your program and MS SQL Server works.

While github.com/alexbrainman/odbc is a thin wrapper around ODBC code written by Microsoft, probably, in C, and super optimised because every man and his dog uses it.

Alex

@sponeil
Copy link
Author

@sponeil sponeil commented Nov 2, 2018

Yes, I understand that. I wasn't bashing the other library, I was merely pointing out that there's more than one reason for me to prefer the ODBC driver for this particular app's needs.

@kardianos
Copy link
Contributor

@kardianos kardianos commented Apr 5, 2019

I suspect this is caused by an issue with the ODBC driver. At the very least, I know of nothing actionable by database/sql to prevent this error. Closing. If you feel this was a mistake, please open a new issue and try to point me to why this is an error with database/sql. Thanks.

@kardianos kardianos closed this Apr 5, 2019
@golang golang locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.