Skip to content

Commit

Permalink
Only materialize the new transaction if it needs it
Browse files Browse the repository at this point in the history
Historically this was unnecessary, because all possible Transaction
instances would start their life unmaterialized. But now, if the new
transaction is a ResetParentTransaction, its materialization state
relies upon its parent, so it may already be done. Indeed, if lazy
transactions are disabled, the parent is almost certainly already
materialized. #materialize! will happily re-run the query without
question (a behaviour we rely upon elsewhere), so in this case it's up
to the caller to check first.
  • Loading branch information
matthewd committed Apr 26, 2022
1 parent 7883100 commit 9a803d3
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,12 @@ def begin_transaction(isolation: nil, joinable: true, _lazy: true)
)
end

if @connection.supports_lazy_transactions? && lazy_transactions_enabled? && _lazy
@has_unmaterialized_transactions = true
else
transaction.materialize!
unless transaction.materialized?
if @connection.supports_lazy_transactions? && lazy_transactions_enabled? && _lazy
@has_unmaterialized_transactions = true
else
transaction.materialize!
end
end
@stack.push(transaction)
transaction
Expand Down
38 changes: 38 additions & 0 deletions activerecord/test/cases/transactions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,44 @@ def test_nested_transactions_skip_excess_savepoints
end
end

def test_nested_transactions_after_disable_lazy_transactions
Topic.connection.disable_lazy_transactions!

capture_sql do
# RealTransaction (begin..commit)
Topic.transaction(requires_new: true) do
# ResetParentTransaction (no queries)
Topic.transaction(requires_new: true) do
Topic.delete_all
# SavepointTransaction (savepoint..release)
Topic.transaction(requires_new: true) do
# ResetParentTransaction (no queries)
Topic.transaction(requires_new: true) do
# no-op
end
end
end
Topic.delete_all
end
end

actual_queries = ActiveRecord::SQLCounter.log_all

expected_queries = [
/BEGIN/i,
/DELETE/i,
/^SAVEPOINT/i,
/^RELEASE/i,
/DELETE/i,
/COMMIT/i,
]

assert_equal expected_queries.size, actual_queries.size
expected_queries.zip(actual_queries) do |expected, actual|
assert_match expected, actual
end
end

if ActiveRecord::Base.connection.prepared_statements
def test_prepared_statement_materializes_transaction
Topic.first
Expand Down

0 comments on commit 9a803d3

Please sign in to comment.