diff options
author | Edouard CHIN <edouard.chin@shopify.com> | 2017-12-12 15:38:10 -0500 |
---|---|---|
committer | Edouard CHIN <edouard.chin@shopify.com> | 2018-01-22 13:02:56 -0500 |
commit | 22a26d249f78d650752057d0a5d82f34a312d041 (patch) | |
tree | 140f0ee8873f2b8b9ebbd243cfc96db35ad94e5a /activerecord/test/cases | |
parent | a681eaf22955734c142609961a6d71746cfa0583 (diff) | |
download | rails-22a26d249f78d650752057d0a5d82f34a312d041.tar.gz rails-22a26d249f78d650752057d0a5d82f34a312d041.tar.bz2 rails-22a26d249f78d650752057d0a5d82f34a312d041.zip |
Build a multi-statement query when inserting fixtures:
- The `insert_fixtures` method can be optimized by making a single multi statement query for all fixtures having the same connection instead of doing a single query per table
- The previous code was bulk inserting fixtures for a single table, making X query for X fixture files
- This patch builds a single **multi statement query** for every tables. Given a set of 3 fixtures (authors, dogs, computers):
```ruby
# before
%w(authors dogs computers).each do |table|
sql = build_sql(table)
connection.query(sql)
end
# after
sql = build_sql(authors, dogs, computers)
connection.query(sql)
```
- `insert_fixtures` is now deprecated, `insert_fixtures_set` is the new way to go with performance improvement
- My tests were done with an app having more than 700 fixtures, the time it takes to insert all of them was around 15s. Using a single multi statement query, it took on average of 8 seconds
- In order for a multi statement to be executed, mysql needs to be connected with the `MULTI_STATEMENTS` [flag](https://dev.mysql.com/doc/refman/5.7/en/c-api-multiple-queries.html), which is done before inserting the fixtures by reconnecting to da the database with the flag declared. Reconnecting to the database creates some caveats:
1. We loose all open transactions; Inside the original code, when inserting fixtures, a transaction is open. Multple delete statements are [executed](https://github.com/rails/rails/blob/a681eaf22955734c142609961a6d71746cfa0583/activerecord/lib/active_record/fixtures.rb#L566) and finally the fixtures are inserted. The problem with this patch is that we need to open the transaction only after we reconnect to the DB otherwise reconnecting drops the open transaction which doesn't commit all delete statements and inserting fixtures doesn't work since we duplicated them (Primary key duplicate exception)...
- In order to fix this problem, the transaction is now open directly inside the `insert_fixtures` method, right after we reconnect to the db
- As an effect, since the transaction is open inside the `insert_fixtures` method, the DELETE statements need to be executed here since the transaction is open later
2. The same problem happens for the `disable_referential_integrity` since we reconnect, the `FOREIGN_KEY_CHECKS` is reset to the original value
- Same solution as 1. , the disable_referential_integrity can be called after we reconnect to the transaction
3. When the multi statement query is executed, no other queries can be performed until we paginate over the set of results, otherwise mysql throws a "Commands out of sync" [Ref](https://dev.mysql.com/doc/refman/5.7/en/commands-out-of-sync.html)
- Iterating over the set of results until `mysql_client.next_result` is false. [Ref](https://github.com/brianmario/mysql2#multiple-result-sets)
- Removed the `active_record.sql "Fixture delete"` notification, the delete statements are now inside the INSERT's one
- On mysql the `max_allowed_packet` is looked up:
1. Before executing the multi-statements query, we check the packet length of each statements, if the packet is bigger than the max_allowed_packet config, an `ActiveRecordError` is raised
2. Otherwise we concatenate the current sql statement into the previous and so on until the packet is `< max_allowed_packet`
Diffstat (limited to 'activerecord/test/cases')
-rw-r--r-- | activerecord/test/cases/adapters/postgresql/array_test.rb | 4 | ||||
-rw-r--r-- | activerecord/test/cases/fixtures_test.rb | 116 |
2 files changed, 119 insertions, 1 deletions
diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 0e9e86f425..7eecd5bc35 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -228,7 +228,9 @@ class PostgresqlArrayTest < ActiveRecord::PostgreSQLTestCase def test_insert_fixtures tag_values = ["val1", "val2", "val3_with_'_multiple_quote_'_chars"] - @connection.insert_fixtures([{ "tags" => tag_values }], "pg_arrays") + assert_deprecated do + @connection.insert_fixtures([{ "tags" => tag_values }], "pg_arrays") + end assert_equal(PgArray.last.tags, tag_values) end diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index b0b63f5203..13034eef22 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -79,6 +79,122 @@ class FixturesTest < ActiveRecord::TestCase ActiveSupport::Notifications.unsubscribe(subscription) end end + + def test_bulk_insert_multiple_table_with_a_multi_statement_query + subscriber = InsertQuerySubscriber.new + subscription = ActiveSupport::Notifications.subscribe("sql.active_record", subscriber) + + create_fixtures("bulbs", "authors", "computers") + + expected_sql = <<-EOS.strip_heredoc.chop + INSERT INTO #{ActiveRecord::Base.connection.quote_table_name("bulbs")} .* + INSERT INTO #{ActiveRecord::Base.connection.quote_table_name("authors")} .* + INSERT INTO #{ActiveRecord::Base.connection.quote_table_name("computers")} .* + EOS + assert_equal 1, subscriber.events.size + assert_match(/#{expected_sql}/, subscriber.events.first) + ensure + ActiveSupport::Notifications.unsubscribe(subscription) + end + + def test_bulk_insert_with_a_multi_statement_query_raises_an_exception_when_any_insert_fails + require "models/aircraft" + + assert_equal false, Aircraft.columns_hash["wheels_count"].null + fixtures = { + "aircraft" => [ + { "name" => "working_aircrafts", "wheels_count" => 2 }, + { "name" => "broken_aircrafts", "wheels_count" => nil }, + ] + } + + assert_no_difference "Aircraft.count" do + assert_raises(ActiveRecord::NotNullViolation) do + ActiveRecord::Base.connection.insert_fixtures_set(fixtures) + end + end + end + end + + if current_adapter?(:Mysql2Adapter) + def test_insert_fixtures_set_raises_an_error_when_max_allowed_packet_is_smaller_than_fixtures_set_size + conn = ActiveRecord::Base.connection + packet_size = 1024 + fixtures = { + "traffic_lights" => [ + { "location" => "US", "state" => ["NY"], "long_state" => ["a" * packet_size] }, + ] + } + + conn.stubs(:max_allowed_packet).returns(packet_size) + + error = assert_raises(ActiveRecord::ActiveRecordError) { conn.insert_fixtures_set(fixtures) } + assert_match(/Fixtures set is too large/, error.message) + end + + def test_insert_fixture_set_when_max_allowed_packet_is_bigger_than_fixtures_set_size + conn = ActiveRecord::Base.connection + packet_size = 1024 + fixtures = { + "traffic_lights" => [ + { "location" => "US", "state" => ["NY"], "long_state" => ["a" * 51] }, + ] + } + + conn.stubs(:max_allowed_packet).returns(packet_size) + + assert_difference "TrafficLight.count" do + conn.insert_fixtures_set(fixtures) + end + end + + def test_insert_fixtures_set_split_the_total_sql_into_two_chunks_smaller_than_max_allowed_packet + subscriber = InsertQuerySubscriber.new + subscription = ActiveSupport::Notifications.subscribe("sql.active_record", subscriber) + conn = ActiveRecord::Base.connection + packet_size = 1024 + fixtures = { + "traffic_lights" => [ + { "location" => "US", "state" => ["NY"], "long_state" => ["a" * 450] }, + ], + "comments" => [ + { "post_id" => 1, "body" => "a" * 450 }, + ] + } + + conn.stubs(:max_allowed_packet).returns(packet_size) + + conn.insert_fixtures_set(fixtures) + assert_equal 2, subscriber.events.size + assert_operator subscriber.events.first.bytesize, :<, packet_size + assert_operator subscriber.events.second.bytesize, :<, packet_size + ensure + ActiveSupport::Notifications.unsubscribe(subscription) + end + + def test_insert_fixtures_set_concat_total_sql_into_a_single_packet_smaller_than_max_allowed_packet + subscriber = InsertQuerySubscriber.new + subscription = ActiveSupport::Notifications.subscribe("sql.active_record", subscriber) + conn = ActiveRecord::Base.connection + packet_size = 1024 + fixtures = { + "traffic_lights" => [ + { "location" => "US", "state" => ["NY"], "long_state" => ["a" * 200] }, + ], + "comments" => [ + { "post_id" => 1, "body" => "a" * 200 }, + ] + } + + conn.stubs(:max_allowed_packet).returns(packet_size) + + assert_difference ["TrafficLight.count", "Comment.count"], +1 do + conn.insert_fixtures_set(fixtures) + end + assert_equal 1, subscriber.events.size + ensure + ActiveSupport::Notifications.unsubscribe(subscription) + end end def test_broken_yaml_exception |