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.

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

Jump to bottom

database/sql: prepared statements are bound to connections, autoreconnection doesn't work #5718

Closed
mattetti opened this issue Jun 16, 2013 · 15 comments
Closed

database/sql: prepared statements are bound to connections, autoreconnection doesn't work #5718

mattetti opened this issue Jun 16, 2013 · 15 comments
Labels
FrozenDueToAge

Comments

@mattetti
Copy link
Contributor 

What steps will reproduce the problem?
Bug originally submitted to one of the MySQL driver project until we realized it was an
actual Go bug.
Original bug report: go-sql-driver/mysql#98

If possible, include a link to a program on play.golang.org.
1.  create a prepared statement
2.  query the statement in a loop
3.  while the queries are running, kill the DB connection

What is the expected output?
The driver should auto-reconnect like it does if instead of sending using prepared
statements I query the database directly.


What do you see instead?
The driver doesn't auto-reconnect.

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
OS X

Which version are you using?  (run 'go version')
go1.1.1 darwin/amd64

Please provide any additional information below.

It looks like the issue is that prepared statements are bound to a connection. In the
case where the connection is severed, the prepared statements don't get a new connection
and keep on failing.
This is a big deal since in real life most DB queries are or should be prepared to avoid
SQL injections and to improve performance. Furthermore many SQL high level libraries
like https://github.com/eaigner/jet rely on prepared statements.
@robpike
Copy link
Contributor

robpike commented Jun 17, 2013

Comment 1:

Labels changed: added priority-later, removed priority-triage.

Owner changed to @bradfitz.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 2:

Labels changed: added go1.2maybe.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 3:

Labels changed: added feature.

@robpike
Copy link
Contributor

robpike commented Aug 30, 2013

Comment 4:

Not for 1.2.

Labels changed: removed go1.2maybe.

@julienschmidt
Copy link
Contributor

Comment 5:

Mailed https://golang.org/cl/14920046
I think this shouldn't be postponed to after Go 1.2.
Not working auto-reconnecting for prepared statements is a serious flaw IMO.
This is a thing the user should not have to care about and the database/sql package
makes this promise.

@adg
Copy link
Contributor

adg commented Oct 21, 2013

Comment 6:

Sorry, would have been great if this had been done months ago when the issue was first
reported. We're well and truly locked down now, only fixes to very serious issues will
be accepted.

@ghost
Copy link

ghost commented Oct 30, 2013

Comment 7:

I have encountered this problem too

@mattetti
Copy link
Contributor Author

Comment 8:

Hopefully it will make it into trunk soon after 1.2, it's indeed quite an annoying bug.

@julienschmidt
Copy link
Contributor

Comment 9:

The current code causes an especially critical bug in stmt.Exec:
http://code.google.com/p/go/source/browse/src/pkg/database/sql/sql.go#1252
The connection is put back to the connection pool even if resultFromStatement returned
an error.
This can lead to nasty panics if the driver does not completely distrust the
database/sql package and assumes that no connections marked by ErrBadConn are reused.
Here is one example case by better0332 where this happened:
go-sql-driver/mysql#142
I'm still of the opinion that this bug should be fixed in Go 1.2. From my point of view
this bug is very critical. My current advise would be not to use prepared statements
with the database/sql package unless the code is manually patched. And certainly not in
production.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 10:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 11:

Labels changed: removed feature.

@dsymonds
Copy link
Contributor

dsymonds commented Dec 2, 2013

Comment 12:

Labels changed: added packagechange.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 13:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 14:

Labels changed: added repo-main.

@bradfitz
Copy link
Contributor

Comment 15:

This issue was closed by revision 762a9d9.

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@mattetti @bradfitz @dsymonds @rsc @julienschmidt @robpike @adg @gopherbot