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: NumInput check before CheckNamedValue could remove unwanted arguments #22630

Closed
tgulacsi opened this issue Nov 8, 2017 · 3 comments
Closed
Assignees
Milestone

Comments

@tgulacsi
Copy link
Contributor

@tgulacsi tgulacsi commented Nov 8, 2017

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

go version go1.9.2 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

linux/amd64

What did you do?

I've implemented an database/sql/driver for Oracle: gopkg.in/goracle.v2
The driver.Stmt.NumInput method returns the (proper) number of required unique arguments.

I'm calling Query with additional arguments, such as the prefetch row count:
db.Query("SELECT :1 FROM DUAL", "arg", goracle.FetchRowCount(1024))

What did you expect to see?

No errors.

What did you see instead?

error: "sql: statement expects 1 inputs; got 2"

Investigation

As I see, the error is returned from https://golang.org/src/database/sql/sql.go#L2280, right before the driver could eliminate unwanted args (fetchRowCount) with CheckNamedValue returning driver.ErrRemoveArgument.

CheckNamedValue is called later, as the next call is to driverArgs, and in it the CheckNamedValue is called (https://golang.org/src/database/sql/convert.go#L191), and the argument number is checked, but this time after CheckNamedValue - so this part is OK, but the previous check in sql.go is too early.

tgulacsi added a commit to go-goracle/goracle that referenced this issue Nov 8, 2017
@tgulacsi
Copy link
Contributor Author

@tgulacsi tgulacsi commented Nov 8, 2017

ErrRemoveArgument has been introduced in a9bf3b2 by @kardianos

@kardianos
Copy link
Contributor

@kardianos kardianos commented Nov 9, 2017

@tgulacsi Tanks for the report. I'll look into and get a fix out soon.

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 9, 2017

Change https://golang.org/cl/76732 mentions this issue: database/sql: check for arg counts after eliminating arguments

@kardianos kardianos added this to the Go1.10 milestone Nov 10, 2017
@kardianos kardianos self-assigned this Nov 10, 2017
@gopherbot gopherbot closed this in 262141a Nov 18, 2017
@golang golang locked and limited conversation to collaborators Nov 18, 2018
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
3 participants
You can’t perform that action at this time.