Leave no open transaction when a transaction helper fails

src/SQLite.jl

@@ -873,15 +873,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
 
@@ -958,7 +967,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
 
@@ -986,7 +995,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")

src/tables.jl

@@ -356,31 +356,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
 

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