diff options
author | Eugene Kenny <elkenny@gmail.com> | 2018-08-13 16:51:54 +0100 |
---|---|---|
committer | Eugene Kenny <elkenny@gmail.com> | 2018-08-13 16:51:54 +0100 |
commit | 0ac81ee6ff3d1625fdbcc40b12c00cbff2208077 (patch) | |
tree | 525e64b50d3eaa9e0d545f170510ae1b9ffd67b1 /activerecord/test | |
parent | f2970a08b57ebcdb9cbf8eec5d10a7f04eb7b9d3 (diff) | |
download | rails-0ac81ee6ff3d1625fdbcc40b12c00cbff2208077.tar.gz rails-0ac81ee6ff3d1625fdbcc40b12c00cbff2208077.tar.bz2 rails-0ac81ee6ff3d1625fdbcc40b12c00cbff2208077.zip |
Omit BEGIN/COMMIT statements for empty transactions
If a transaction is opened and closed without any queries being run, we
can safely omit the `BEGIN` and `COMMIT` statements, as they only exist
to modify the connection's behaviour inside the transaction. This
removes the overhead of those statements when saving a record with no
changes, which makes workarounds like `save if changed?` unnecessary.
This implementation buffers transactions inside the transaction manager
and materializes them the next time the connection is used. For this to
work, the adapter needs to guard all connection use with a call to
`materialize_transactions`. Because of this, adapters must opt in to get
this new behaviour by implementing `supports_lazy_transactions?`.
If `raw_connection` is used to get a reference to the underlying
database connection, the behaviour is disabled and transactions are
opened eagerly, as we can't know how the connection will be used.
However when the connection is checked back into the pool, we can assume
that the application won't use the reference again and reenable lazy
transactions. This prevents a single `raw_connection` call from
disabling lazy transactions for the lifetime of the connection.
Diffstat (limited to 'activerecord/test')
8 files changed, 82 insertions, 3 deletions
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 59b99351d1..67734d24d7 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -10,6 +10,7 @@ module ActiveRecord class AdapterTest < ActiveRecord::TestCase def setup @connection = ActiveRecord::Base.connection + @connection.materialize_transactions end ## diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb index 726f58d58e..0c0e2a116e 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -170,6 +170,8 @@ class Mysql2ConnectionTest < ActiveRecord::Mysql2TestCase end def test_logs_name_show_variable + ActiveRecord::Base.connection.materialize_transactions + @subscriber.logged.clear @connection.show_variable "foo" assert_equal "SCHEMA", @subscriber.logged[0][1] end diff --git a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb index 308ad1d854..afd422881b 100644 --- a/activerecord/test/cases/adapters/postgresql/active_schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/active_schema_test.rb @@ -4,6 +4,8 @@ require "cases/helper" class PostgresqlActiveSchemaTest < ActiveRecord::PostgreSQLTestCase def setup + ActiveRecord::Base.connection.materialize_transactions + ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.class_eval do def execute(sql, name = nil) sql end end diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb index 54b0dde7dc..70aa189893 100644 --- a/activerecord/test/cases/adapters/postgresql/connection_test.rb +++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb @@ -15,8 +15,9 @@ module ActiveRecord def setup super @subscriber = SQLSubscriber.new - @subscription = ActiveSupport::Notifications.subscribe("sql.active_record", @subscriber) @connection = ActiveRecord::Base.connection + @connection.materialize_transactions + @subscription = ActiveSupport::Notifications.subscribe("sql.active_record", @subscriber) end def teardown diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index f0126fdb0d..ae2597adc8 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -44,6 +44,7 @@ class LogSubscriberTest < ActiveRecord::TestCase def setup @old_logger = ActiveRecord::Base.logger Developer.primary_key + ActiveRecord::Base.connection.materialize_transactions super ActiveRecord::LogSubscriber.attach_to(:active_record) end diff --git a/activerecord/test/cases/test_case.rb b/activerecord/test/cases/test_case.rb index 409b07e56c..40947767f3 100644 --- a/activerecord/test/cases/test_case.rb +++ b/activerecord/test/cases/test_case.rb @@ -31,6 +31,7 @@ module ActiveRecord end def capture_sql + ActiveRecord::Base.connection.materialize_transactions SQLCounter.clear_log yield SQLCounter.log_all.dup @@ -48,6 +49,7 @@ module ActiveRecord def assert_queries(num = 1, options = {}) ignore_none = options.fetch(:ignore_none) { num == :any } + ActiveRecord::Base.connection.materialize_transactions SQLCounter.clear_log x = yield the_log = ignore_none ? SQLCounter.log_all : SQLCounter.log diff --git a/activerecord/test/cases/transaction_isolation_test.rb b/activerecord/test/cases/transaction_isolation_test.rb index eaafd13360..8bcf121f15 100644 --- a/activerecord/test/cases/transaction_isolation_test.rb +++ b/activerecord/test/cases/transaction_isolation_test.rb @@ -11,7 +11,7 @@ unless ActiveRecord::Base.connection.supports_transaction_isolation? test "setting the isolation level raises an error" do assert_raises(ActiveRecord::TransactionIsolationError) do - Tag.transaction(isolation: :serializable) {} + Tag.transaction(isolation: :serializable) { Topic.connection.materialize_transactions } end end end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 46463ac414..b13cf88c00 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -575,7 +575,7 @@ class TransactionTest < ActiveRecord::TestCase assert_called(Topic.connection, :rollback_db_transaction) do e = assert_raise RuntimeError do Topic.transaction do - # do nothing + Topic.connection.materialize_transactions end end assert_equal "OH NOES", e.message @@ -943,6 +943,76 @@ class TransactionTest < ActiveRecord::TestCase connection.drop_table "transaction_without_primary_keys", if_exists: true end + def test_empty_transaction_is_not_materialized + assert_no_queries do + Topic.transaction {} + end + end + + def test_unprepared_statement_materializes_transaction + assert_sql(/BEGIN/i, /COMMIT/i) do + Topic.transaction { Topic.where("1=1").first } + end + end + + if ActiveRecord::Base.connection.prepared_statements + def test_prepared_statement_materializes_transaction + Topic.first + + assert_sql(/BEGIN/i, /COMMIT/i) do + Topic.transaction { Topic.first } + end + end + end + + def test_savepoint_does_not_materialize_transaction + assert_no_queries do + Topic.transaction do + Topic.transaction(requires_new: true) {} + end + end + end + + def test_raising_does_not_materialize_transaction + assert_raise(RuntimeError) do + assert_no_queries do + Topic.transaction { raise } + end + end + end + + def test_accessing_raw_connection_materializes_transaction + assert_sql(/BEGIN/i, /COMMIT/i) do + Topic.transaction { Topic.connection.raw_connection } + end + end + + def test_accessing_raw_connection_disables_lazy_transactions + Topic.connection.raw_connection + + assert_sql(/BEGIN/i, /COMMIT/i) do + Topic.transaction {} + end + end + + def test_checking_in_connection_reenables_lazy_transactions + connection = Topic.connection_pool.checkout + connection.raw_connection + Topic.connection_pool.checkin connection + + assert_no_queries do + connection.transaction {} + end + end + + def test_transactions_can_be_manually_materialized + assert_sql(/BEGIN/i, /COMMIT/i) do + Topic.transaction do + Topic.connection.materialize_transactions + end + end + end + private %w(validation save destroy).each do |filter| |