Isolation testing only applies to on-disk

db/db.go

@@ -582,7 +582,7 @@ func (db *DB) queryWithConn(req *command.Request, xTime bool, conn *sql.Conn) ([
 		var rows *command.QueryRows
 		var err error
 
-		readOnly, err := db.StmtReadOnly(sql)
+		readOnly, err := db.StmtReadOnlyWithConn(sql, conn)
 		if err != nil {
 			stats.Add(numQueryErrors, 1)
 			rows = &command.QueryRows{
@@ -738,7 +738,7 @@ func (db *DB) Request(req *command.Request, xTime bool) ([]*command.ExecuteQuery
 			continue
 		}
 
-		ro, err := db.StmtReadOnly(ss)
+		ro, err := db.StmtReadOnlyWithConn(ss, conn)
 		if err != nil {
 			eqResponse = append(eqResponse, &command.ExecuteQueryResponse{
 				Result: &command.ExecuteQueryResponse_Error{
@@ -927,6 +927,17 @@ func (db *DB) Dump(w io.Writer) error {
 // As per https://www.sqlite.org/c3ref/stmt_readonly.html, this function
 // may not return 100% correct results, but should cover most scenarios.
 func (db *DB) StmtReadOnly(sql string) (bool, error) {
+	conn, err := db.roDB.Conn(context.Background())
+	if err != nil {
+		return false, err
+	}
+	defer conn.Close()
+	return db.StmtReadOnlyWithConn(sql, conn)
+}
+
+// StmtReadOnlyWithConn returns whether the given SQL statement is read-only, using
+// the given connection.
+func (db *DB) StmtReadOnlyWithConn(sql string, conn *sql.Conn) (bool, error) {
 	var readOnly bool
 	f := func(driverConn interface{}) error {
 		c := driverConn.(*sqlite3.SQLiteConn)
@@ -940,11 +951,6 @@ func (db *DB) StmtReadOnly(sql string) (bool, error) {
 		return nil
 	}
 
-	conn, err := db.roDB.Conn(context.Background())
-	if err != nil {
-		return false, err
-	}
-	defer conn.Close()
 	if err := conn.Raw(f); err != nil {
 		return false, err
 	}

db/db_common_test.go

@@ -803,7 +803,8 @@ func testSimpleRequest(t *testing.T, db *DB) {
 }
 
 // Test_SimpleRequestTx tests that a transaction is rolled back when an error occurs, and that
-// subsequent statements after the failed statement are not processed.
+// subsequent statements after the failed statement are not processed. This also checks that
+// the code which checks if the statement is a query or not works when holding a transaction.
 func testSimpleRequestTx(t *testing.T, db *DB) {
 	mustExecute(db, `CREATE TABLE foo (id INTEGER NOT NULL PRIMARY KEY, name TEXT)`)
 	mustExecute(db, `INSERT INTO foo(id, name) VALUES(1, "fiona")`)
@@ -883,55 +884,6 @@ func testUniqueConstraints(t *testing.T, db *DB) {
 	}
 }
 
-// testConnectionIsolation test that ISOLATION behavior of on-disk databases doesn't
-// change unexpectedly.
-func testConnectionIsolation(t *testing.T, db *DB) {
-	r, err := db.ExecuteStringStmt("CREATE TABLE foo (id INTEGER NOT NULL PRIMARY KEY, name TEXT)")
-	if err != nil {
-		t.Fatalf("failed to create table: %s", err.Error())
-	}
-	if exp, got := `[{}]`, asJSON(r); exp != got {
-		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
-	}
-
-	r, err = db.ExecuteStringStmt("BEGIN")
-	if err != nil {
-		t.Fatalf("failed to create table: %s", err.Error())
-	}
-	if exp, got := `[{}]`, asJSON(r); exp != got {
-		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
-	}
-
-	r, err = db.ExecuteStringStmt(`INSERT INTO foo(name) VALUES("fiona")`)
-	if err != nil {
-		t.Fatalf("error executing insertion into table: %s", err.Error())
-	}
-	if exp, got := `[{"last_insert_id":1,"rows_affected":1}]`, asJSON(r); exp != got {
-		t.Fatalf("unexpected results for execute, expected %s, got %s", exp, got)
-	}
-
-	q, err := db.QueryStringStmt("SELECT * FROM foo")
-	if err != nil {
-		t.Fatalf("failed to query empty table: %s", err.Error())
-	}
-	if exp, got := `[{"columns":["id","name"],"types":["integer","text"]}]`, asJSON(q); exp != got {
-		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
-	}
-
-	_, err = db.ExecuteStringStmt("COMMIT")
-	if err != nil {
-		t.Fatalf("error executing insertion into table: %s", err.Error())
-	}
-
-	q, err = db.QueryStringStmt("SELECT * FROM foo")
-	if err != nil {
-		t.Fatalf("failed to query empty table: %s", err.Error())
-	}
-	if exp, got := `[{"columns":["id","name"],"types":["integer","text"],"values":[[1,"fiona"]]}]`, asJSON(q); exp != got {
-		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
-	}
-}
-
 func testPartialFail(t *testing.T, db *DB) {
 	_, err := db.ExecuteStringStmt("CREATE TABLE foo (id INTEGER NOT NULL PRIMARY KEY, name TEXT)")
 	if err != nil {
@@ -1345,7 +1297,6 @@ func Test_DatabaseCommonOperations(t *testing.T) {
 		{"SimpleRequestTx", testSimpleRequestTx},
 		{"CommonTableExpressions", testCommonTableExpressions},
 		{"UniqueConstraints", testUniqueConstraints},
-		{"ConnectionIsolation", testConnectionIsolation},
 		{"PartialFail", testPartialFail},
 		{"Serialize", testSerialize},
 		{"Dump", testDump},

db/db_ondisk_test.go

@@ -67,3 +67,60 @@ func Test_FileCreationOnDisk(t *testing.T) {
 		t.Fatalf("failed to close database: %s", err.Error())
 	}
 }
+
+// Test_ConnectionIsolationOnDisk test that ISOLATION behavior of on-disk databases doesn't
+// change unexpectedly.
+func Test_ConnectionIsolationOnDisk(t *testing.T) {
+	dir := t.TempDir()
+	dbPath := path.Join(dir, "test_db")
+
+	db, err := Open(dbPath, false)
+	if err != nil {
+		t.Fatalf("failed to open new database: %s", err.Error())
+	}
+
+	r, err := db.ExecuteStringStmt("CREATE TABLE foo (id INTEGER NOT NULL PRIMARY KEY, name TEXT)")
+	if err != nil {
+		t.Fatalf("failed to create table: %s", err.Error())
+	}
+	if exp, got := `[{}]`, asJSON(r); exp != got {
+		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
+	}
+
+	r, err = db.ExecuteStringStmt("BEGIN")
+	if err != nil {
+		t.Fatalf("failed to create table: %s", err.Error())
+	}
+	if exp, got := `[{}]`, asJSON(r); exp != got {
+		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
+	}
+
+	r, err = db.ExecuteStringStmt(`INSERT INTO foo(name) VALUES("fiona")`)
+	if err != nil {
+		t.Fatalf("error executing insertion into table: %s", err.Error())
+	}
+	if exp, got := `[{"last_insert_id":1,"rows_affected":1}]`, asJSON(r); exp != got {
+		t.Fatalf("unexpected results for execute, expected %s, got %s", exp, got)
+	}
+
+	q, err := db.QueryStringStmt("SELECT * FROM foo")
+	if err != nil {
+		t.Fatalf("failed to query empty table: %s", err.Error())
+	}
+	if exp, got := `[{"columns":["id","name"],"types":["integer","text"]}]`, asJSON(q); exp != got {
+		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
+	}
+
+	_, err = db.ExecuteStringStmt("COMMIT")
+	if err != nil {
+		t.Fatalf("error executing insertion into table: %s", err.Error())
+	}
+
+	q, err = db.QueryStringStmt("SELECT * FROM foo")
+	if err != nil {
+		t.Fatalf("failed to query empty table: %s", err.Error())
+	}
+	if exp, got := `[{"columns":["id","name"],"types":["integer","text"],"values":[[1,"fiona"]]}]`, asJSON(q); exp != got {
+		t.Fatalf("unexpected results for query, expected %s, got %s", exp, got)
+	}
+}