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

error on ?NNN binding #584

Open
win-t opened this issue Jun 3, 2018 · 2 comments
Open

error on ?NNN binding #584

win-t opened this issue Jun 3, 2018 · 2 comments

Comments

@win-t
Copy link

win-t commented Jun 3, 2018

This one work:

if _, err := db.Exec(`
	UPDATE tempstorage SET value = ?, expired = ? WHERE key = ?;
	INSERT INTO tempstorage(key, value, expired) SELECT ?, ?, ? WHERE changes() = 0;
`, value, expiredOnMilis, key, key, value, expiredOnMilis); err != nil {
	panic(err)
}

but this one doesn't:

if _, err := db.Exec(`
	UPDATE tempstorage SET value = ?2, expired = ?3 WHERE key = ?1;
	INSERT INTO tempstorage(key, value, expired) SELECT ?1, ?2, ?3 WHERE changes() = 0;
`, key, value, expiredOnMilis); err != nil {
	panic(err)
}

I don't know if this error is from upstream, please investigate.

Thanks

@gjrtimmer
Copy link
Collaborator

Reference: #473

@rittneje
Copy link
Collaborator

rittneje commented Jan 6, 2019

The problem here is that from SQLite's perspective, the UPDATE and the INSERT are two totally separate statements. Consequently, they each have parameters numbered 1 through 3 with respect to the SQLite APIs. However, Go considers the whole thing to be a single query, with parameters numbered 1 through 6. To make this work, this library "consumes" the first three parameters in the first query, and then the second three parameters in the second query.

Unfortunately, this causes an issue in your second example, in which you've manually specified the parameters indices. You are trying to reuse parameters between the two statements, so when this library "consumes" them for the first statement, it ends up failing in the second statement. (I'm assuming you are getting a "not enough args to execute query" error.)

To fix this, this library should no longer attempt to "consume" parameters in each loop iteration. Instead, use sqlite3_bind_parameter_name for each parameter in the current statement to determine whether the parameter is anonymous. If so, bind the right parameter from the Go list at that SQLite index. To calculate the Go index, I believe it should be the sum of sqlite3_bind_parameter_count for all the preceding statements, plus the SQLite index of the parameter in that statement.

For example, consider your first query.

if _, err := db.Exec(`
	UPDATE tempstorage SET value = ?, expired = ? WHERE key = ?;
	INSERT INTO tempstorage(key, value, expired) SELECT ?, ?, ? WHERE changes() = 0;
`, value, expiredOnMilis, key, key, value, expiredOnMilis); err != nil {
	panic(err)
}

Let's follow the algorithm I proposed to find the Go index for the first ? in the INSERT query. The preceding UPDATE query has a parameter count of 3. The ? is parameter index 1 in the INSERT. Therefore, we should use the 4th parameter in the Go list, which is the second occurrence of key, as expected.

Note that this will not work if someone tries to mix parameter forms. For example:

if _, err := db.Exec(`
	UPDATE tempstorage SET value = ?002 WHERE key = ?001;
	UPDATE tempstorage SET expired = ?003 WHERE key = ?001;
	DELETE FROM tempstorage WHERE key = ?;
`, key, value, expiredOnMilis, otherKey); err != nil {
	panic(err)
}

In this case, the algorithm would translate the ? in the DELETE to 2+2+1=5, which is incorrect. You'd need a more complicated algorithm to deal with this, but I'm not sure how common this use case really is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants