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

Jump to bottom

database/sql: statements created with conn.PrepareContext are re-prepared in transactions #28130

Closed
RaduBerinde opened this issue Oct 10, 2018 · 3 comments
Closed

database/sql: statements created with conn.PrepareContext are re-prepared in transactions #28130

RaduBerinde opened this issue Oct 10, 2018 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.12

Comments

@RaduBerinde
Copy link
Contributor

I am trying to write a sql client that prepares some statements and then uses them in transactions. I am observing that Tx.StmtContext re-prepares each statement every time (defeating the purpose of using prepared statements, at least from a performance standpoint).

This is another incarnation of #15606. The fix for that issue addresses the case where we create the statement via DB.PrepareContext. However, I am using Conn.PrepareContext. I am then running transactions on the same Conn, which triggers the code path below because stmt.cg is not nil:

// [sql.go:2105, go 1.11]
	if stmt.closed || stmt.cg != nil {
		// If the statement has been closed or already belongs to a
		// transaction, we can't reuse it in this connection.
		// Since tx.StmtContext should never need to be called with a
		// Stmt already belonging to tx, we ignore this edge case and
		// re-prepare the statement in this case. No need to add
		// code-complexity for this.

Note that (as an alternative) using Db.PrepareContext once for the application is problematic for other reasons - we have multiple parallel workers and they contend on some internal mutexes in Rows objects. Perhaps I should use Db.PrepareContext once per each worker, even though I already have a specific Conn for each one?

I am using go 1.11.

CC @adams-sarah @kardianos @johto
@kardianos kardianos self-assigned this Oct 12, 2018
@bcmills
Copy link
Contributor

bcmills commented Oct 23, 2018

CC @bradfitz @kevinburke

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 23, 2018
@bcmills bcmills added this to the Go1.12 milestone Oct 23, 2018
@bradfitz bradfitz changed the title database/sql: statments created with conn.PrepareContext are re-prepared in transactions database/sql: statements created with conn.PrepareContext are re-prepared in transactions Oct 23, 2018
@kardianos
Copy link
Contributor

@RaduBerinde If you start a Tx from a Conn, what you have are two views into the same database session. If you prepare a stmt on Conn, begin a tran, then execute the stmt on the conn, it will execute within the transaction (because the session is still open). We still prevent executing multiple commands at the same time from conn or tx because we wrap the driver conn in a mutex.

The short story, just don't use "StmtContext" and it should just work today.

I attempted to see if I could get this workflow working as you described, but the implicit stmt pool code is too complicated for me without spending many hours re-reading it. I was unable to find a simple fix that would work. So in this case, just don't use "StmtContext" on Tx, and just remember that you are operating on a database session, regardless of the objects you use on client space.

@RaduBerinde
Copy link
Contributor Author

Thanks for the workaround. This limitation and workaround should be documented if it won't be fixed though.

@golang golang locked and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
4 participants
@kardianos @bcmills @gopherbot @RaduBerinde