diff --git a/src/SQLite.jl b/src/SQLite.jl index a51c150..0747a05 100644 --- a/src/SQLite.jl +++ b/src/SQLite.jl @@ -879,15 +879,24 @@ DBInterface.transaction(f, db::DB) = transaction(f, db) @inline function transaction(f::Function, db::DB) # generate a random name for the savepoint name = string("SQLITE", Random.randstring(10)) + outermost = !intransaction(db) transaction(db, name) try - f() + result = f() + commit(db, name) + return result catch - rollback(db, name) + # A failed statement can end the whole transaction (`ON CONFLICT ROLLBACK`, + # `SQLITE_FULL`), and a failed outermost RELEASE (`SQLITE_BUSY`) leaves it open. + if intransaction(db) + if outermost + rollback(db) + else + rollback(db, name) + commit(db, name) # savepoints are not released on rollback + end + end rethrow() - finally - # savepoints are not released on rollback - commit(db, name) end end @@ -964,7 +973,7 @@ function createindex!( "CREATE $u INDEX $exists $(esc_id(index)) ON $(esc_id(table)) ($(esc_id(cols)))", ) end - execute(db, "ANALYZE $index") + execute(db, "ANALYZE $(esc_id(index))") return end @@ -992,7 +1001,7 @@ function removeduplicates!( "DELETE FROM $(esc_id(table)) WHERE _ROWID_ NOT IN (SELECT max(_ROWID_) from $(esc_id(table)) GROUP BY $(colsstr));", ) end - execute(db, "ANALYZE $table") + execute(db, "ANALYZE $(esc_id(table))") end include("tables.jl") diff --git a/src/tables.jl b/src/tables.jl index 3270afe..fb0b56e 100644 --- a/src/tables.jl +++ b/src/tables.jl @@ -358,31 +358,34 @@ function load!( ) handle = _get_stmt_handle(stmt) # start a transaction for inserting rows - DBInterface.transaction(db) do - if row === nothing - state = iterate(rows) - state === nothing && return - row, st = state - end - while true - Tables.eachcolumn(sch, row) do val, col, _ - bind!(stmt, col, val) + try + DBInterface.transaction(db) do + if row === nothing + state = iterate(rows) + state === nothing && return + row, st = state end - r = Base.@lock db.lock GC.@preserve row C.sqlite3_step(handle) - if r == C.SQLITE_DONE - Base.@lock db.lock C.sqlite3_reset(handle) - elseif r != C.SQLITE_ROW - e = sqliteexception(db, stmt) - Base.@lock db.lock C.sqlite3_reset(handle) - throw(e) + while true + Tables.eachcolumn(sch, row) do val, col, _ + bind!(stmt, col, val) + end + r = Base.@lock db.lock GC.@preserve row C.sqlite3_step(handle) + if r == C.SQLITE_DONE + Base.@lock db.lock C.sqlite3_reset(handle) + elseif r != C.SQLITE_ROW + e = sqliteexception(db, stmt) + Base.@lock db.lock C.sqlite3_reset(handle) + throw(e) + end + state = iterate(rows, st) + state === nothing && break + row, st = state end - state = iterate(rows, st) - state === nothing && break - row, st = state end + finally + _close_stmt!(stmt) end - _close_stmt!(stmt) - analyze && execute(db, "ANALYZE $name") + analyze && execute(db, "ANALYZE $(esc_id(name))") return name end diff --git a/test/runtests.jl b/test/runtests.jl index b06fcb8..5b31a2d 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -909,9 +909,68 @@ end SQLite.commit(db) @test SQLite.intransaction(db) == false end + + @testset "Failed transactions leave no open transaction" begin + mktempdir() do dir + path = joinpath(dir, "busy.sqlite") + db = SQLite.DB(path) + reader = SQLite.DB(path) + try + SQLite.execute(db, "CREATE TABLE t (k INTEGER PRIMARY KEY)") + SQLite.execute(db, "INSERT INTO t VALUES (1)") + # ON CONFLICT ROLLBACK ends the transaction before the helper rolls back + err = try + SQLite.load!((k = [2, 1],), db, "t"; on_conflict = "ROLLBACK") + catch e + e + end + @test err isa SQLiteException && occursin("UNIQUE", err.msg) + err = try + DBInterface.transaction(db) do + SQLite.execute(db, "INSERT OR ROLLBACK INTO t VALUES (1)") + end + catch e + e + end + @test err isa SQLiteException && occursin("UNIQUE", err.msg) + @test !SQLite.intransaction(db) + # An unfinished read on another connection makes the commit fail with SQLITE_BUSY + q = DBInterface.execute(reader, "SELECT k FROM t") + @test first(q).k == 1 + @test_throws SQLiteException DBInterface.transaction(db) do + SQLite.execute(db, "INSERT INTO t VALUES (10)") + end + @test !SQLite.intransaction(db) + @test_throws SQLiteException SQLite.load!((k = [11],), db, "t") + @test !SQLite.intransaction(db) + # load! finalized its INSERT statement + @test SQLite.C.sqlite3_next_stmt(db.handle, C_NULL) == C_NULL + DBInterface.close!(q) + SQLite.execute(db, "INSERT INTO t VALUES (20)") + @test columntable( + DBInterface.execute(reader, "SELECT k FROM t ORDER BY k"), + ).k == [1, 20] + finally + close(reader) + close(db) + end + end + end end @testset "load! / drop!" begin + @testset "ANALYZE quotes table and index names" begin + db = SQLite.DB() + @test SQLite.load!((a = [1, 1, 2],), db, "my table"; analyze = true) == "my table" + SQLite.createindex!(db, "my table", "my index", "a"; unique = false) + SQLite.removeduplicates!(db, "my table", ["a"]) + @test columntable( + DBInterface.execute(db, "SELECT a FROM \"my table\" ORDER BY a"), + ).a == [1, 2] + @test "my table" in columntable(DBInterface.execute(db, "SELECT tbl FROM sqlite_stat1")).tbl + close(db) + end + @testset "Remove Duplicates" begin setup_clean_test_db() do db db = SQLite.DB() #In case the order of tests is changed