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 @@ module ActiveRecord
                :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 @@ module ActiveRecord
 
     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 @@ module ActiveRecord
             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 @@ module ActiveRecord
       # 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 @@ module ActiveRecord
             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 @@ module ActiveRecord
         # 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 @@ module ActiveRecord
           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 @@ module ActiveRecord
 
         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 @@ module ActiveRecord
 
         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 @@ module ActiveRecord
           end
 
           materialize_transactions
+          mark_transaction_written_if_write(sql)
 
           log(sql, name) do
             ActiveSupport::Dependencies.interlock.permit_concurrent_loads do
@@ -38,6 +39,7 @@ module ActiveRecord
           end
 
           materialize_transactions
+          mark_transaction_written_if_write(sql)
 
           type_casted_binds = type_casted_binds(binds)
 
@@ -113,6 +115,7 @@ module ActiveRecord
             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 @@ class TransactionTest < ActiveRecord::TestCase
     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