zombiezen / go-sqlite

Low-level Go interface to SQLite 3
https://pkg.go.dev/zombiezen.com/go/sqlite
ISC License
741 stars 18 forks source link

postgres style $1/$2 are treated as named arguments #63

Closed Perusae closed 8 months ago

Perusae commented 9 months ago

Not sure if its a go-sqlite issue or modernc.org/sqlite but using binding with:

err := sqlitex.Execute(conn, sqlStatement, &sqlitex.ExecOptions{ResultFunc: func(stmt *sqlite.Stmt) error {}, Args: argList})

Where sqlStatement is:

SELECT EXISTS( SELECT 1 FROM x WHERE z = $2 AND y = $1

And argList is:

[8, 4]

Results in Z=8, Y = 4, instead of the correct Z = 4, Y = 8... In other words, they are not treated like positioned variables, as they are supposed to.

Try doing a update with the postgresql style $1 style. It will not error but simply take the $2/$1 as if you simply wrote WHERE z = ? AND y = ?... Resulting in the wrong positioning and updates the wrong elements or non at all. As a result, you end up with a silent bug that will creep all over your data.

Sqlite:

In the SQL statement text input to [sqlite3_prepare_v2()](https://www.sqlite.org/c3ref/prepare.html) and its variants, literals may be replaced by a [parameter](https://www.sqlite.org/lang_expr.html#varparam) that matches one of following templates:

?
?NNN
:VVV
@VVV
$VVV

Switching to the ?NNN

SELECT EXISTS( SELECT 1 FROM x WHERE z = ?2 AND y = ?1 [8, 4]

Works without issue and respects the order / naming ...

If you come from a other sqlite Go library that respect this "postgres style", this can create issues that are hard to trace back. And WILL result in data corruption as the wrong data gets update, delete or inserted or not deleted/updated at all as this are not direct errors.

zombiezen commented 9 months ago

This looks like a difference between PostgreSQL and SQLite. According the official SQLite docs:

A dollar-sign followed by an identifier name also holds a spot for a named parameter (emphasis mine) with the name $AAAA.

So I would expect that this would work (untested):

const sqlStatement = `SELECT EXISTS( SELECT 1 FROM x WHERE z = $2 AND y = $1);`
err := sqlitex.Execute(conn, sqlStatement, &sqlitex.ExecOptions{
  ResultFunc: func(stmt *sqlite.Stmt) error { /* ... */ },
  Named: map[string]any{
    "$1": 8,
    "$2": 4,
  },
})

If you come from a other sqlite Go library that respect this "postgres style", this can create issues that are hard to trace back.

Can you help point me to which Go SQLite library you are referring to? If emulating the positional parameter style is behavior another library has, it would help me weigh whether we should shim this in.

Perusae commented 9 months ago

Might have been Matts go-Sqlite3 version. Been ages when we rewrote the codebase to sqlite and our zombiezen/go-sqlite update is 5+ months old.

Maybe it never supported it and we never ran into this issue as our order just matched all the time. It was only recently that data issues in our database (see ref on first post) triggered several day investigation, and this "behavior" issue come to light.

The solution was simply to replace all instances of $1 with ?1...

Our issue is simply how easy it is to creates a silent bug if your using $1 and expect ?1 behavior. Maybe a warning is better to be put in place then changing the default behavior of go-sqlite?

zombiezen commented 8 months ago

I'm closing this as "won't fix", since the general position of this library is to be largely the same as SQLite. I understand this design decision has caused some frustration on your end, but I can't see a way to improve this situation. Thanks for the feedback, and I'll keep it in mind if I see the same sorts of issues arise in the future.