tursodatabase / go-libsql

libSQL API for Go
MIT License
82 stars 14 forks source link

Exec method does not support queries that return results #28

Open nalgeon opened 4 months ago

nalgeon commented 4 months ago

Some sqlite pragmas return a value. For example, mmap_size (running in sqlite shell):

sqlite> pragma mmap_size=1024;
1024

The libsql driver's Exec method fails with an "Execute returned rows" for such pragmas. In fact, it fails for any query that returns a value (such as select 42).

I would argue that this is not an expected behavior. It's perfectly normal for an Exec query to return a value. Other drivers (such as mattn/go-sqlite3 and modernc.org/sqlite) allow this.

Code to reproduce:

package main

import (
    "database/sql"
    "fmt"
    "log"

    _ "github.com/tursodatabase/go-libsql"
)

func main() {
    db, err := sql.Open("libsql", "file:data.db")
    if err != nil {
        log.Fatal(err)
    }
    defer db.Close()

    // You can replace the query with `select 42` to get the same error.
    res, err := db.Exec(`pragma mmap_size=1024`)
    fmt.Println("res", res)
    fmt.Println("err", err)
}

Result:

res <nil>
err failed to execute query pragma mmap_size=1024
error code = 2: Error executing statement: Execute returned rows
haaawk commented 4 months ago

Have you consider using Query?

nalgeon commented 4 months ago

Not looking for a workaround; I reported the issue so that the driver behaves as expected.

haaawk commented 4 months ago

The driver behavies as expected. Execute returns number of affected rows and last affected rowid Query returns rows. All our drivers behave this way. So does rusqlite -> https://docs.rs/rusqlite/latest/rusqlite/struct.Connection.html#method.execute, go-sqlite3 and database/sql -> https://pkg.go.dev/database/sql/driver#Result

nalgeon commented 4 months ago

The libSQL driver returns an error. It's described in the issue. The error is not expected in this situation, and other Go SQLite drivers handle the described situation just fine.

haaawk commented 4 months ago

Ok. I'm still not sure what is a desired behaviour here but reopening the issue for now.

nalgeon commented 4 months ago

I'd say the expected behavior is to return a non-nil sql.Result and not return an error (like other drivers do):

package main

import (
    "database/sql"
    "fmt"
    "log"

    _ "github.com/mattn/go-sqlite3"
)

func main() {
    db, err := sql.Open("sqlite3", "file:data.db")
    if err != nil {
        log.Fatal(err)
    }
    defer db.Close()

    res, err := db.Exec(`pragma mmap_size=1024`)
    fmt.Println("res", res)
    fmt.Println("err", err)
}
res {0x14000146000 0x1400011c040}
err <nil>
haaawk commented 4 months ago

This is more complicated in case of turso which can have not only local backend but also a remote one. Ideally what you're suggesting would be best but I have to think about all available backends before being sure.

nalgeon commented 4 months ago

I see, thank you for clarifying that!