activerecord: No warning for return out of transaction block without writes

It doesn't matter if the transaction is rolled back or committed if it
wasn't written to, so we can avoid warning about a breaking change.

activerecord/CHANGELOG.md

@@ -253,7 +253,7 @@
 
     *Eugene Kenny*
 
-*   Deprecate using `return`, `break` or `throw` to exit a transaction block.
+*   Deprecate using `return`, `break` or `throw` to exit a transaction block after writes.
 
     *Dylan Thacker-Smith*
 

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

@@ -323,6 +323,13 @@ def transaction(requires_new: nil, isolation: nil, joinable: true)
                :commit_transaction, :rollback_transaction, :materialize_transactions,
                :disable_lazy_transactions!, :enable_lazy_transactions!, to: :transaction_manager
 
+      def mark_transaction_written_if_write(sql) # :nodoc:
+        transaction = current_transaction
+        if transaction.open?
+          transaction.written ||= write_query?(sql)
+        end
+      end
+
       def transaction_open?
         current_transaction.open?
       end

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

@@ -75,6 +75,7 @@ def add_record(record, _ = true); end
 
     class Transaction #:nodoc:
       attr_reader :connection, :state, :savepoint_name, :isolation_level
+      attr_accessor :written
 
       def initialize(connection, isolation: nil, joinable: true, run_commit_callbacks: false)
         @connection = connection
@@ -320,7 +321,7 @@ def within_new_transaction(isolation: nil, joinable: true)
             if Thread.current.status == "aborting"
               rollback_transaction
             else
-              unless completed
+              if !completed && transaction.written
                 ActiveSupport::Deprecation.warn(<<~EOW)
                   Using `return`, `break` or `throw` to exit a transaction block is
                   deprecated without replacement. If the `throw` came from

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -191,6 +191,7 @@ def clear_cache! # :nodoc:
       # Executes the SQL statement in the context of this connection.
       def execute(sql, name = nil)
         materialize_transactions
+        mark_transaction_written_if_write(sql)
 
         log(sql, name) do
           ActiveSupport::Dependencies.interlock.permit_concurrent_loads do

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

@@ -154,6 +154,7 @@ def exec_stmt_and_free(sql, name, binds, cache_stmt: false)
             end
 
             materialize_transactions
+            mark_transaction_written_if_write(sql)
 
             # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been
             # made since we established the connection

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

@@ -12,6 +12,7 @@ def explain(arel, binds = [])
         # Queries the database and returns the results in an Array-like object
         def query(sql, name = nil) #:nodoc:
           materialize_transactions
+          mark_transaction_written_if_write(sql)
 
           log(sql, name) do
             ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
@@ -39,6 +40,7 @@ def execute(sql, name = nil)
           end
 
           materialize_transactions
+          mark_transaction_written_if_write(sql)
 
           log(sql, name) do
             ActiveSupport::Dependencies.interlock.permit_concurrent_loads do

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -651,6 +651,7 @@ def execute_and_clear(sql, name, binds, prepare: false)
 
         def exec_no_cache(sql, name, binds)
           materialize_transactions
+          mark_transaction_written_if_write(sql)
 
           # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been
           # made since we established the connection
@@ -666,6 +667,7 @@ def exec_no_cache(sql, name, binds)
 
         def exec_cache(sql, name, binds)
           materialize_transactions
+          mark_transaction_written_if_write(sql)
           update_typemap_for_default_timezone
 
           stmt_key = prepare_statement(sql, binds)

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

@@ -24,6 +24,7 @@ def execute(sql, name = nil) #:nodoc:
           end
 
           materialize_transactions
+          mark_transaction_written_if_write(sql)
 
           log(sql, name) do
             ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
@@ -38,6 +39,7 @@ def exec_query(sql, name = nil, binds = [], prepare: false)
           end
 
           materialize_transactions
+          mark_transaction_written_if_write(sql)
 
           type_casted_binds = type_casted_binds(binds)
 
@@ -113,6 +115,7 @@ def execute_batch(statements, name = nil)
             end
 
             materialize_transactions
+            mark_transaction_written_if_write(sql)
 
             log(sql, name) do
               ActiveSupport::Dependencies.interlock.permit_concurrent_loads do

activerecord/test/cases/transactions_test.rb

@@ -177,6 +177,14 @@ def test_deprecation_on_ruby_timeout
     assert Topic.find(1).approved?, "First should have been approved"
   end
 
+  def test_early_return_from_transaction
+    assert_not_deprecated do
+      @first.with_lock do
+        break
+      end
+    end
+  end
+
   def test_number_of_transactions_in_commit
     num = nil