Support nested transactions via savepoints

lib/ecto_libsql.ex

@@ -302,17 +302,42 @@ defmodule EctoLibSql do
     |> Enum.map(fn [_full, name] -> name end)
   end
 
+  # DBConnection drives nested transactions with `mode: :savepoint`. SQLite allows
+  # duplicate savepoint names and resolves RELEASE/ROLLBACK TO against the most
+  # recent match, so one fixed name nests correctly - the same approach postgrex
+  # takes with "postgrex_savepoint".
+  @savepoint "ecto_libsql_savepoint"
+
   @impl true
   @doc """
   Begins a new database transaction.
 
   The transaction behaviour (deferred/immediate/exclusive) can be controlled
   via options passed to the Native module.
   """
-  def handle_begin(_opts, state) do
-    case EctoLibSql.Native.begin(state) do
-      {:ok, new_state} -> {:ok, :begin, new_state}
-      {:error, reason} -> {:error, reason, state}
+  def handle_begin(opts, state) do
+    # `:mode` is overloaded here: DBConnection sets `:transaction` or `:savepoint`,
+    # while this adapter's own API uses it for the libSQL transaction mode
+    # (`:deferred`, `:immediate`, `:exclusive`, `:read_only`). Only `:savepoint` is
+    # intercepted; everything else, including an absent mode, begins as before so
+    # the transaction modes keep working.
+    case {Keyword.get(opts, :mode), state.trx_id} do
+      {:savepoint, trx_id} when is_binary(trx_id) ->
+        case EctoLibSql.Native.create_savepoint(state, @savepoint) do
+          :ok -> {:ok, %EctoLibSql.Result{}, state}
+          {:error, reason} -> {:error, savepoint_error(reason), state}
+        end
+
+      {:savepoint, nil} ->
+        # A savepoint outside a transaction is not something SQLite can honour.
+        # Report status and let DBConnection decide rather than emitting bad SQL.
+        {:idle, state}
+
+      _other ->
+        case EctoLibSql.Native.begin(state) do
+          {:ok, new_state} -> {:ok, :begin, new_state}
+          {:error, reason} -> {:error, reason, state}
+        end
     end
   end
 
@@ -328,15 +353,24 @@ defmodule EctoLibSql do
   The state must contain a valid transaction ID. For embedded replicas with
   auto-sync enabled, this will also trigger a sync to the remote database.
   """
-  def handle_commit(_opts, state) do
-    case EctoLibSql.Native.commit(
-           %EctoLibSql.State{conn_id: conn_id, trx_id: _trx_id, mode: mode} = state
-         ) do
-      {:ok, _} ->
-        {:ok, %EctoLibSql.Result{}, %EctoLibSql.State{conn_id: conn_id, mode: mode}}
-
-      {:error, reason} ->
-        {:disconnect, reason, state}
+  def handle_commit(opts, state) do
+    if Keyword.get(opts, :mode, :transaction) == :savepoint do
+      # Releasing a savepoint leaves the enclosing transaction open, so the
+      # state - trx_id included - carries through untouched.
+      case EctoLibSql.Native.release_savepoint_by_name(state, @savepoint) do
+        :ok -> {:ok, %EctoLibSql.Result{}, state}
+        {:error, reason} -> {:error, savepoint_error(reason), state}
+      end
+    else
+      case EctoLibSql.Native.commit(
+             %EctoLibSql.State{conn_id: conn_id, trx_id: _trx_id, mode: mode} = state
+           ) do
+        {:ok, _} ->
+          {:ok, %EctoLibSql.Result{}, %EctoLibSql.State{conn_id: conn_id, mode: mode}}
+
+        {:error, reason} ->
+          {:disconnect, reason, state}
+      end
     end
   end
 
@@ -347,16 +381,32 @@ defmodule EctoLibSql do
   Discards all changes made within the transaction and returns the connection
   to autocommit mode.
   """
-  def handle_rollback(_opts, %EctoLibSql.State{conn_id: conn_id, trx_id: _trx_id} = state) do
-    case EctoLibSql.Native.rollback(state) do
-      {:ok, _} ->
-        {:ok, %EctoLibSql.Result{}, %EctoLibSql.State{conn_id: conn_id, trx_id: nil}}
+  def handle_rollback(opts, %EctoLibSql.State{conn_id: conn_id, trx_id: _trx_id} = state) do
+    if Keyword.get(opts, :mode, :transaction) == :savepoint do
+      # ROLLBACK TO leaves the savepoint itself active (SQLite keeps it so it can
+      # be rolled back to again), so it has to be released explicitly afterwards
+      # or the next savepoint of the same name nests inside this one.
+      with :ok <- EctoLibSql.Native.rollback_to_savepoint_by_name(state, @savepoint),
+           :ok <- EctoLibSql.Native.release_savepoint_by_name(state, @savepoint) do
+        {:ok, %EctoLibSql.Result{}, state}
+      else
+        {:error, reason} -> {:error, savepoint_error(reason), state}
+      end
+    else
+      case EctoLibSql.Native.rollback(state) do
+        {:ok, _} ->
+          {:ok, %EctoLibSql.Result{}, %EctoLibSql.State{conn_id: conn_id, trx_id: nil}}
 
-      {:error, reason} ->
-        {:disconnect, reason, state}
+        {:error, reason} ->
+          {:disconnect, reason, state}
+      end
     end
   end
 
+  defp savepoint_error(reason) when is_binary(reason), do: %RuntimeError{message: reason}
+  defp savepoint_error(%{__exception__: true} = reason), do: reason
+  defp savepoint_error(reason), do: %RuntimeError{message: inspect(reason)}
+
   @impl true
   @doc """
   Closes the query. Currently a no-op as queries are stateless.

test/ecto_sandbox_savepoint_test.exs

@@ -0,0 +1,115 @@
+defmodule EctoLibSql.EctoSandboxSavepointTest do
+  @moduledoc """
+  Coverage for `Ecto.Adapters.SQL.Sandbox`, which is how most Ecto applications run
+  their entire test suite.
+
+  The sandbox holds one transaction open for the duration of each test and rolls it
+  back afterwards, so every `Repo.transaction/1` in application code runs nested and
+  is issued by DBConnection with `mode: :savepoint`. Without SAVEPOINT support in
+  `handle_begin/2` that mode is discarded, a plain BEGIN is issued inside the open
+  transaction, and the test fails with "cannot start a transaction within a
+  transaction" - meaning no transactional application code can be tested at all.
+  """
+  use ExUnit.Case, async: false
+
+  import Ecto.Query
+
+  alias Ecto.Adapters.SQL.Sandbox
+
+  defmodule SandboxRepo do
+    use Ecto.Repo, otp_app: :ecto_libsql, adapter: Ecto.Adapters.LibSql
+  end
+
+  defmodule Item do
+    use Ecto.Schema
+
+    schema "sandbox_items" do
+      field(:num, :integer)
+    end
+  end
+
+  setup do
+    unique_id = :erlang.unique_integer([:positive])
+    test_db = "z_ecto_libsql_test-sandbox_savepoint-#{unique_id}.db"
+
+    {:ok, pid} =
+      SandboxRepo.start_link(
+        database: test_db,
+        pool_size: 1,
+        pool: Sandbox,
+        name: SandboxRepo
+      )
+
+    Sandbox.mode(SandboxRepo, :manual)
+    owner = Sandbox.start_owner!(SandboxRepo, shared: true)
+
+    Ecto.Adapters.SQL.query!(SandboxRepo, """
+    CREATE TABLE IF NOT EXISTS sandbox_items (
+      id INTEGER PRIMARY KEY AUTOINCREMENT,
+      num INTEGER
+    )
+    """)
+
+    on_exit(fn ->
+      Sandbox.stop_owner(owner)
+
+      # Process.alive?/1 is a point-in-time check: stopping the owner above can take
+      # the repo supervisor down while we are still on this line, and Supervisor.stop/1
+      # then exits with :noproc. Catch it rather than failing an otherwise passing test.
+      if Process.alive?(pid) do
+        try do
+          :ok = Supervisor.stop(pid)
+        catch
+          :exit, _ -> :ok
+        end
+      end
+
+      Process.sleep(50)
+      for f <- Path.wildcard(test_db <> "*"), do: File.rm(f)
+    end)
+
+    :ok
+  end
+
+  defp nums, do: SandboxRepo.all(from(i in Item, order_by: i.num, select: i.num))
+
+  test "a transaction can be opened inside the sandbox transaction at all" do
+    assert {:ok, :done} =
+             SandboxRepo.transaction(fn ->
+               SandboxRepo.insert!(%Item{num: 1})
+               :done
+             end)
+
+    assert nums() == [1]
+  end
+
+  test "rolling back only discards work done inside the savepoint" do
+    SandboxRepo.insert!(%Item{num: 1})
+
+    assert {:error, :nope} =
+             SandboxRepo.transaction(fn ->
+               SandboxRepo.insert!(%Item{num: 2})
+               SandboxRepo.rollback(:nope)
+             end)
+
+    # The write made before the savepoint survives, and the sandbox transaction is
+    # still usable afterwards - which is the whole point of rolling back to a
+    # savepoint rather than aborting the enclosing transaction.
+    assert nums() == [1]
+    SandboxRepo.insert!(%Item{num: 3})
+    assert nums() == [1, 3]
+  end
+
+  test "an exception inside the transaction is contained the same way" do
+    SandboxRepo.insert!(%Item{num: 1})
+
+    assert_raise RuntimeError, "boom", fn ->
+      SandboxRepo.transaction(fn ->
+        SandboxRepo.insert!(%Item{num: 2})
+        raise "boom"
+      end)
+    end
+
+    assert nums() == [1]
+  end
+end