Refactor SQLite3Adapter to simplify transaction management

Followup: rails#52428

This adapter was overly complex following rails#37798.

I'm also not convinced storing the `read_uncommited` value in the
execution state was really valid, it's a connection property.

We can assume outside of transaction, `read_uncommited` is set to
the default, and simply reset it at the end of any transaction
that did change it.

This makes SQLite3Adapter much less of a snowflake.

Author: byroot

activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb

@@ -435,6 +435,15 @@ def begin_isolated_db_transaction(isolation)
         raise ActiveRecord::TransactionIsolationError, "adapter does not support setting transaction isolation"
       end
 
+      # Hook point called after an isolated DB transaction is committed
+      # or rolled back.
+      # Most adapters don't need to implement anything because the isolation
+      # level is set on a per transaction basis.
+      # But some databases like SQLite set it on a per connection level
+      # and need to explicitly reset it after commit or rollback.
+      def reset_isolation_level
+      end
+
       # Commits the transaction (and turns on auto-committing).
       def commit_db_transaction()   end
 

activerecord/lib/active_record/connection_adapters/abstract/transaction.rb

@@ -476,13 +476,19 @@ def restart
       end
 
       def rollback
-        connection.rollback_db_transaction if materialized?
+        if materialized?
+          connection.rollback_db_transaction
+          connection.reset_isolation_level if isolation_level
+        end
         @state.full_rollback!
         @instrumenter.finish(:rollback) if materialized?
       end
 
       def commit
-        connection.commit_db_transaction if materialized?
+        if materialized?
+          connection.commit_db_transaction
+          connection.reset_isolation_level if isolation_level
+        end
         @state.full_commit!
         @instrumenter.finish(:commit) if materialized?
       end

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -225,6 +225,8 @@ def begin_db_transaction # :nodoc:
       end
 
       def begin_isolated_db_transaction(isolation) # :nodoc:
+        # From MySQL manual: The [SET TRANSACTION] statement applies only to the next single transaction performed within the session.
+        # So we don't need to implement #reset_isolation_level
         internal_execute("SET TRANSACTION ISOLATION LEVEL #{transaction_isolation_levels.fetch(isolation)}", "TRANSACTION", allow_retry: true, materialize_transactions: false)
         begin_db_transaction
       end

activerecord/lib/active_record/connection_adapters/sqlite3/database_statements.rb

@@ -34,21 +34,11 @@ def begin_db_transaction # :nodoc:
         end
 
         def commit_db_transaction # :nodoc:
-          log("commit transaction", "TRANSACTION") do
-            with_raw_connection(allow_retry: true, materialize_transactions: false) do |conn|
-              conn.commit
-            end
-          end
-          reset_read_uncommitted
+          internal_execute("COMMIT TRANSACTION", "TRANSACTION", allow_retry: true, materialize_transactions: false)
         end
 
         def exec_rollback_db_transaction # :nodoc:
-          log("rollback transaction", "TRANSACTION") do
-            with_raw_connection(allow_retry: true, materialize_transactions: false) do |conn|
-              conn.rollback
-            end
-          end
-          reset_read_uncommitted
+          internal_execute("ROLLBACK TRANSACTION", "TRANSACTION", allow_retry: true, materialize_transactions: false)
         end
 
         # https://stackoverflow.com/questions/17574784
@@ -66,23 +56,22 @@ def execute(...) # :nodoc:
           super&.to_a
         end
 
+        def reset_isolation_level # :nodoc:
+          internal_execute("PRAGMA read_uncommitted=#{@previous_read_uncommitted}", "TRANSACTION", allow_retry: true, materialize_transactions: false)
+          @previous_read_uncommitted = nil
+        end
+
         private
           def internal_begin_transaction(mode, isolation)
             if isolation
               raise TransactionIsolationError, "SQLite3 only supports the `read_uncommitted` transaction isolation level" if isolation != :read_uncommitted
               raise StandardError, "You need to enable the shared-cache mode in SQLite mode before attempting to change the transaction isolation level" unless shared_cache?
             end
 
-            log("begin #{mode} transaction", "TRANSACTION") do
-              with_raw_connection(allow_retry: true, materialize_transactions: false) do |conn|
-                if isolation
-                  ActiveSupport::IsolatedExecutionState[:active_record_read_uncommitted] = conn.get_first_value("PRAGMA read_uncommitted")
-                  conn.read_uncommitted = true
-                end
-                result = conn.transaction(mode)
-                verified!
-                result
-              end
+            internal_execute("BEGIN #{mode} TRANSACTION", allow_retry: true, materialize_transactions: false)
+            if isolation
+              @previous_read_uncommitted = query_value("PRAGMA read_uncommitted")
+              internal_execute("PRAGMA read_uncommitted=ON", "TRANSACTION", allow_retry: true, materialize_transactions: false)
             end
           end
 
@@ -142,7 +131,7 @@ def reset_read_uncommitted
           def execute_batch(statements, name = nil)
             sql = combine_multi_statements(statements)
 
-            log(sql, name) do |notification_payload|
+            log(sql, name) do
               with_raw_connection do |conn|
                 conn.execute_batch2(sql)
                 verified!

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -120,6 +120,7 @@ def initialize(...)
         end
 
         @last_affected_rows = nil
+        @previous_read_uncommitted = nil
         @config[:strict] = ConnectionAdapters::SQLite3Adapter.strict_strings_by_default unless @config.key?(:strict)
         @connection_parameters = @config.merge(
           database: @config[:database].to_s,