SQLite Adapter doesn't roll back transaction on error

[egoodhall]

In the SQLite adapter's StatementExec method, the opened transaction doesn't get rolled back if compat.ExecContext returns an error. This came up in a personal project after hitting a constraint violation - all further write calls to the DB would return database is locked errors until I restarted the application.

I was able to reproduce the behavior using this test:

db_test.go

package db_test

import (
	"path/filepath"
	"testing"

	"github.com/mattn/go-sqlite3"
	"github.com/upper/db/v4"
	"github.com/upper/db/v4/adapter/sqlite"
)

type A struct {
	A int64 `db:"a"`
}

type B struct {
	B int64 `db:"b"`
}

func checkErr(t *testing.T, err error) {
	if err != nil {
		t.Fatal(err)
	}
}

func TestDatabaseIsLocked(t *testing.T) {
	dbFile := filepath.Join(t.TempDir(), "test.db")

	// Open DB
	sess, err := sqlite.Open(sqlite.ConnectionURL{
		Database: dbFile,
		Options: map[string]string{
			"foreign_keys": "on",
		},
	})
	checkErr(t, err)
	defer sess.Close()

	// Set up scenario
	_, err = sess.SQL().Exec("CREATE TABLE IF NOT EXISTS table1 (a INTEGER NOT NULL PRIMARY KEY)")
	checkErr(t, err)
	_, err = sess.Collection("table1").Insert(&A{1})
	checkErr(t, err)
	_, err = sess.SQL().Exec("CREATE TABLE IF NOT EXISTS table2 (b INTEGER NOT NULL PRIMARY KEY)")
	checkErr(t, err)
	_, err = sess.Collection("table2").Insert(&B{2})
	checkErr(t, err)

	// Trigger constraint violation on table 1
	_, err = sess.Collection("table1").Insert(&A{1})
	if err.(sqlite3.Error).Code != sqlite3.ErrConstraint {
		panic(err)
	}

	// Run tests
	t.Run("Read from table 1", testReadTable1(sess))
	t.Run("Read from table 2", testReadTable2(sess))
	t.Run("Insert into table 1", testInsertTable1(sess))
	t.Run("Insert into table 2", testInsertTable2(sess))
}

func testReadTable1(sess db.Session) func(t *testing.T) {
	return func(t *testing.T) {
		err := sess.Collection("table1").Find().One(new(A))
		if err != nil {
			t.Error(err)
		}
	}
}

func testReadTable2(sess db.Session) func(t *testing.T) {
	return func(t *testing.T) {
		err := sess.Collection("table2").Find().One(new(B))
		if err != nil {
			t.Error(err)
		}
	}
}

func testInsertTable1(sess db.Session) func(t *testing.T) {
	return func(t *testing.T) {
		_, err := sess.Collection("table1").Insert(&A{3})
		if err != nil {
			t.Error(err)
		}
	}
}

func testInsertTable2(sess db.Session) func(t *testing.T) {
	return func(t *testing.T) {
		_, err := sess.Collection("table2").Insert(&B{3})
		if err != nil {
			t.Error(err)
		}
	}
}

with the following output:

~/src/emm035/playground $ go test -v -count 1 .
--- FAIL: TestActions (20.81s)
    --- FAIL: TestActions/Insert_into_table_1 (10.39s)
        db_test.go:85: database is locked
    --- FAIL: TestActions/Insert_into_table_2 (10.41s)
        db_test.go:94: database is locked
FAIL
FAIL    github.com/emm035/playground  23.884s
FAIL

Adding a rollback statement to the error handling branch here seemed to stop the locking in my application and the test is passing:

~/src/emm035/playground $ go test -v -count 1 .
ok      github.com/emm035/playground  0.114s

I'd be more than happy to submit a PR with the change, but wanted to make sure this is fixing the actual problem and not just obscuring another issue 🙂