aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'master' into add_database_exist_methodGuillermo Iguaran2019-07-0847-176/+421
|\
| * When DATABASE_URL is specified don't trample envs that use a url: keyWill Jessop2019-07-082-5/+24
| | | | | | | | fixes #36610
| * Merge pull request #36616 from kamipo/dont_use_alias_for_grouped_fieldRyuta Kamizono2019-07-082-1/+8
| |\ | | | | | | Do not use aliases in GROUP BY clause
| | * Do not use aliases in GROUP BY clauseRyuta Kamizono2019-07-082-1/+8
| | | | | | | | | | | | | | | | | | | | | It appears that Oracle does not allow using aliases in GROUP BY clause unlike ORDER BY clause. Fixes #36613.
| * | Should `Regexp.escape` quoted table name in regexRyuta Kamizono2019-07-087-115/+51
| |/ | | | | | | | | It is for agnostic test case, since quoted table name may include `.` for all adapters, and `[` / `]` for sqlserver adapter.
| * MySQL: Fix schema dumping `enum` and `set` columns correctlyRyuta Kamizono2019-07-056-7/+55
| | | | | | | | | | | | | | | | | | `enum` and `set` are typed cast as `:string`, but currently the `:string` type is incorrectly reused for schema dumping. A cast type on columns is not always the same with `sql_type`, this fixes schema dumping `enum` and `set` columns to use `sql_type` instead of `type` correctly.
| * Add "SCHEMA" to the query in `configure_connection` like as other adaptersRyuta Kamizono2019-06-281-1/+1
| | | | | | | | | | | | | | This makes to be able to ignore the query in `assert_queries` even if accidentally reconnected a connection. https://buildkite.com/rails/rails/builds/61917#4c49187a-3173-4d5c-8a8d-d65768f5bfc9/1000-1799
| * warning: instance variable @serial not initialized (#36556)utilum2019-06-281-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduced in bba7c63a663b073034f4c73f0d59655751694e5a Before: ``` $ TESTOPTS="-n=/test_yaml_dump_and_load/" bundle exec rake test:postgresql :scisors: ... :scisors: Using postgresql Run options: -n=/test_yaml_dump_and_load/ --seed 36896 /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized /home/u/code/rails/activerecord/lib/active_record/connection_adapters/postgresql/column.rb:15: warning: instance variable @serial not initialized . Finished in 0.195325s, 5.1197 runs/s, 35.8376 assertions/s. 1 runs, 7 assertions, 0 failures, 0 errors, 0 skips ``` Co-authored-by: Ryuta Kamizono <kamipo@gmail.com>
| * Merge pull request #36565 from rails/fix-url-configsEileen M. Uchitelle2019-06-273-2/+33
| |\ | | | | | | Fix broken url configs
| | * Fix broken url configseileencodes2019-06-273-2/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR is to fix #36559 but I also found other issues that haven't been reported. The check for `(config.size == 1 && config.values.all? { |v| v.is_a? String })` was naive. The only reason this passed was because we had tests that had single hash size configs, but that doesn't mean we don't want to create a hash config in other cases. So this now checks for `config["database"] || config["adapter"] || ENV["DATABASE_URL"]`. In the end for url configs we still get a UrlConfig but we need to pass through the HashConfig to create the right kind of UrlConfig. The UrlConfig's are really complex and I don't necessarily understand everything that's needed in order to act the same as Rails 5.2. I edited the connection handler test to demonstrate how the previous implementation was broken when checking config size. Now old and new tests pass so I think this is closer to 5.2. Fixes #36559
| * | Address to "DEPRECATION WARNING: Uniqueness validator will no longer enforce ↵Ryuta Kamizono2019-06-281-1/+1
| |/ | | | | | | | | | | case sensitive comparison in Rails 6.1" Caused by #36210.
| * Merge pull request #36560 from eileencodes/warn-if-database-yml-cant-be-readEileen M. Uchitelle2019-06-272-9/+20
| |\ | | | | | | Warn if we can't read the yaml to create database tasks
| | * Load initial database.yml once, and warn if we can't create taskseileencodes2019-06-272-9/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For multiple databases we attempt to generate the tasks by reading the database.yml before the Rails application is booted. This means that we need to strip out ERB since it could be reading Rails configs. In some cases like https://github.com/rails/rails/issues/36540 the ERB is too complex and we can't overwrite with the DummyCompilier we used in https://github.com/rails/rails/pull/35497. For the complex causes we simply issue a warning that says we couldn't infer the database tasks from the database.yml. While working on this I decided to update the code to only load the database.yml once initially so that we avoid having to issue the same warning multiple times. Note that this had no performance impact in my testing and is merely for not having to save the error off somewhere. Also this feels cleaner. Note that this will not break running tasks that exist, it will just mean that tasks for multi-db like `db:create:other_db` will not be generated. If the database.yml is actually unreadable it will blow up during normal rake task calls. Fixes #36540
| * | `length(title)` is a safe SQL string since #36448Ryuta Kamizono2019-06-261-2/+2
| |/
| * Merge pull request #36210 from ↵Rafael França2019-06-244-1/+49
| |\ | | | | | | | | | | | | vishaltelangre/raise-record-invalid-when-associations-fail-to-save-due-to-uniqueness-failure Fix: ActiveRecord::RecordInvalid is not raised when an associated record fails to #save! due to uniqueness validation failure
| | * Fix: ActiveRecord::RecordInvalid is not raised when an associated record ↵Vishal Telangre2019-05-104-1/+49
| | | | | | | | | | | | | | | | | | | | | | | | fails to #save! due to uniqueness validation failure Add tests Fix tests failing due to introduction of uniquness rule added to Book model
| * | Schema Cache: extract deduplication commonalityKasper Timm Hansen2019-06-211-10/+15
| | |
| * | Also deduplicate schema cache data when using the init_with interfaceJean Boussier2019-06-211-6/+6
| | |
| * | Merge pull request #36526 from yahonda/test_statement_cache_with_in_clause_pgRyuta Kamizono2019-06-211-1/+1
| |\ \ | | | | | | | | Address test_statement_cache_with_in_clause failure
| | * | Address test_statement_cache_with_in_clause failure due to nondeterministic ↵Yasuo Honda2019-06-201-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | sort order This failure is occasional, does not always reproduce. ```ruby $ cd activerecord $ bundle exec rake test_postgresql ... snip ... ....F Failure: ActiveRecord::BindParameterTest#test_statement_cache_with_in_clause [/home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:97]: Expected: [1, 3] Actual: [3, 1] rails test home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:93 ```
| * | | Revert schema dumper to use strings rather than integerseileencodes2019-06-203-3/+4
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I think we should change this, but not in 6-0-stable since that's already in RC and I was trying to only make changes that won't require any app changes. This reverts a portion of https://github.com/rails/rails/pull/36439 that made all schema migration version numbers get dumped as an integer. While it doesn't _really_ matter it did change behavior. We should bring this back in 6.1 with a deprecation.
| * | Merge pull request #36520 from kamipo/test_case_for_deterministic_orderRyuta Kamizono2019-06-201-0/+6
| |\ \ | | | | | | | | Add test cases to ensure deterministic order for ordinal methods
| | * | Add test cases to ensure deterministic order for ordinal methodsRyuta Kamizono2019-06-191-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | Before 1340498d2, `order` with no-op value (e.g. `nil`, `""`) had broken the contract of ordinal methods, which returns a result deterministic ordered.
| * | | Merge pull request #36518 from Shopify/drop-schema-cache-column-hashKasper Timm Hansen2019-06-191-8/+5
| |\ \ \ | | | | | | | | | | Stop serializing and parsing columns_hash in Active Record schema caches
| | * | | Stop serializing and parsing columns_hash in Active Record schema cachesJean Boussier2019-06-191-8/+5
| | |/ /
| * / / Better error message for calling columns_hashGuilherme Mansur2019-06-194-0/+19
| |/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a record does not have a table name, as in the case for a record with `self.abstract_class = true` and no `self.table_name` set the error message raises a cryptic: "ActiveRecord::StatementInvalid: Could not find table ''" this patch now raises a new `TableNotSpecified Error` Fixes: #36274 Co-Authored-By: Eugene Kenny <elkenny@gmail.com>
| * | Merge pull request #35891 from Shopify/schema-cache-deduplicationKasper Timm Hansen2019-06-1912-16/+123
| |\ \ | | | | | | | | Deduplicate various Active Record schema cache structures
| | * | Deduplicate various Active Record schema cache structuresJean Boussier2019-06-0312-16/+123
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Real world database schemas contain a lot of duplicated data. Some column names like `id`, `created_at` etc can easily be repeated hundreds of times. Same for SqlTypeMetada, most database will contain only a limited number of possible combinations. This result in a lot of wasted memory. The idea here is to make these data sctructures immutable, use a registry to substitute similar instances with pre-existing ones.
| * | | Avoid redundant `time.getutc` call if it is already utc time objectRyuta Kamizono2019-06-181-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently `type.serialize` and `connection.{quote|type_cast}` for a time object always does `time.getutc` call regardless of whether it is already utc time object or not, that duplicated proccess (`connection.type_cast(type.serialize(time))`) allocates extra/useless time objects for each type casting. This avoids that redundant `time.getutc` call if it is already utc time object. In the case of a model has timestamps (`created_at` and `updated_at`), it avoids 6,000 time objects allocation for 1,000 times `model.save`. ```ruby ObjectSpace::AllocationTracer.setup(%i{path line type}) pp ObjectSpace::AllocationTracer.trace { 1_000.times { User.create } }.select { |k, _| k[0].end_with?("quoting.rb", "time_value.rb") } ``` Before (c104bfe424e6cebe9c8e85a38515327a6c88b1f8): ``` {["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 203, :T_ARRAY]=>[1004, 0, 778, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 220, :T_STRING]=>[2, 0, 2, 1, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 209, :T_ARRAY]=>[8, 0, 8, 1, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 57, :T_ARRAY]=>[4, 0, 4, 1, 1, 0], ["~/rails/activemodel/lib/active_model/type/helpers/time_value.rb", 17, :T_DATA]=>[4000, 0, 3096, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 120, :T_DATA]=>[2000, 0, 1548, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 126, :T_STRING]=>[4000, 0, 3096, 0, 1, 0]} ``` After (this change): ``` {["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 203, :T_ARRAY]=>[1004, 0, 823, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 220, :T_STRING]=>[2, 0, 2, 1, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 209, :T_ARRAY]=>[8, 0, 8, 1, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 57, :T_ARRAY]=>[4, 0, 4, 1, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb", 126, :T_STRING]=>[2000, 0, 1638, 0, 1, 0]} ```
* | | | Add database_exists? method to connection adaptersGuilherme Mansur2019-06-178-0/+71
|/ / / | | | | | | | | | | | | | | | | | | | | | When SQLite connects it will silently create a database if the database does not exist. This behaviour causes different issues because of inconsistent behaviour between adapters: #36383, #32914. This commit adds a `database_exists?` method as a way to check the database without creating it. This is a stepping stone to fully resolving the above issues.
* | | PostgreSQL: Fix GROUP BY with ORDER BY virtual count attributeRyuta Kamizono2019-06-175-12/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | GROUP BY with virtual count attribute is invalid for almost all databases, but it is valid for PostgreSQL, and it had worked until Rails 5.2.2, so it is a regression for Rails 5.2.3 (caused by 311f001). I can't find perfectly solution for fixing this for now, but I would not like to break existing apps, so I decided to allow referencing virtual count attribute in ORDER BY clause when GROUP BY aggrigation (it partly revert the effect of 311f001) to fix the regression #36022. Fixes #36022.
* | | Remove unused `Arel::Attributes.for`Ryuta Kamizono2019-06-154-70/+1
| | | | | | | | | | | | `Arel::Attributes.for` is no longer used since https://github.com/rails/arel/pull/196.
* | | No allocation `Arel::Visitors::ToSql#visit`Ryuta Kamizono2019-06-154-33/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Each `visit o, collector` allocates one extra array due to receiving args by splat array. https://github.com/rails/rails/blob/2c3332cc4c0fa77dbe2e13e8a792f80fbd8f4ad3/activerecord/lib/arel/visitors/visitor.rb#L27-L29 Currently 1,000 times `User.where(id: 1).to_sql` allocates 13,000 arrays in `visitor.accept`. This avoids receiving args by splat array, it makes `visitor.accept` no array allocation. ```ruby ObjectSpace::AllocationTracer.setup(%i{path line type}) pp ObjectSpace::AllocationTracer.trace { 1_000.times { User.where(id: 1).to_sql } }.select { |k, _| k[2] == :T_ARRAY && k[0].end_with?("visitor.rb", "to_sql.rb") } ``` Before (2c3332cc4c0fa77dbe2e13e8a792f80fbd8f4ad3): ``` {["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 18, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb", 11, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/visitor.rb", 12, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 788, :T_ARRAY]=>[3000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 794, :T_ARRAY]=>[3000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 156, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 443, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 603, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 611, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0]} ``` After (this change): ``` {} ```
* | | Should find last created recordRyuta Kamizono2019-06-151-3/+3
| | | | | | | | | | | | | | | | | | | | | Tables in tests are not always empty so `klass.first` does not always find last created record. Fixes #36479.
* | | Ensure to reset actually used `@connection.schema_migration`'s table nameRyuta Kamizono2019-06-151-4/+4
| | | | | | | | | | | | https://buildkite.com/rails/rails/builds/61744#f12cc6cf-7458-4131-917a-9735615f6259/999-1010
* | | Fix `test_schema_names` to include "hint_plan" schemaRyuta Kamizono2019-06-151-1/+5
| | |
* | | Move while_preventing_writes from conn to handlereileencodes2019-06-148-49/+111
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we put the `while_preventing_writes` on the connection then the middleware that sends reads to the primary and ensures they can't write will not work. The `while_preventing_writes` will only be applied to the connection which it's called on - which in the case of the middleware is Ar::Base. This worked fine if you called it directly like `OtherDbConn.connection.while_preventing_writes` but Rails didn't have a way of knowing you wanted to call it on all the connections. The change here moves the `while_preventing_writes` method from the connection to the handler so that it can block writes to all queries for that handler. This will apply to all the connections associated with that handler.
* | | Merge pull request #36439 from ↵Eileen M. Uchitelle2019-06-1415-146/+407
|\ \ \ | | | | | | | | | | | | | | | | eileencodes/move-schema-migration-to-migration-context Move SchemaMigration to migration_context
| * | | Move SchemaMigration to migration_contexteileencodes2019-06-1415-146/+407
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR moves the `schema_migration` to `migration_context` so that we can access the `schema_migration` per connection. This does not change behavior of the SchemaMigration if you are using one database. This also does not change behavior of any public APIs. `Migrator` is private as is `MigrationContext` so we can change these as needed. We now need to pass a `schema_migration` to `Migrator` so that we can run migrations on the right connection outside the context of a rake task. The bugs this fixes were discovered while debugging the issues around the SchemaCache on initialization with multiple database. It was clear that `get_all_versions` wouldn't work without these changes outside the context of a rake task (because in the rake task we establish a connection and change AR::Base.connection to the db we're running on). Because the `SchemaCache` relies on the `SchemaMigration` information we need to make sure we store it per-connection rather than on ActiveRecord::Base. [Eileen M. Uchitelle & Aaron Patterson]
* | | | Merge pull request #36484 from ↵Ryuta Kamizono2019-06-141-3/+3
|\ \ \ \ | |/ / / |/| | | | | | | | | | | albertoalmagro/alberto/reverse-column-is-reversible [ci skip] Update docs as `remove_column` can be reversed
| * | | [ci skip] Update docs as `remove_column` can be reversedAlberto Almagro2019-06-141-3/+3
| | | | | | | | | | | | | | | | | | | | As `remove_column` can be reversed when a type is provided this example was not accurate anymore.
* | | | Fix rubocop violationsRyuta Kamizono2019-06-141-2/+2
| | | |
* | | | Allocation on demand in transactionsRyuta Kamizono2019-06-141-20/+35
|/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently 1,000 transactions creates 10,000 objects regardless whether it is necessary or not. This makes allocation on demand in transactions, now 1,000 transactions creates required 5,000 objects only by default. ```ruby ObjectSpace::AllocationTracer.setup(%i{path line type}) pp ObjectSpace::AllocationTracer.trace { 1_000.times { User.create } }.select { |k, _| k[0].end_with?("transaction.rb") } ``` Before (95d038f): ``` {["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 209, :T_HASH]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 210, :T_OBJECT]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 210, :T_HASH]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 80, :T_OBJECT]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 8, :T_ARRAY]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 81, :T_ARRAY]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 289, :T_STRING]=>[1000, 0, 714, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 116, :T_ARRAY]=>[1000, 0, 714, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 120, :T_ARRAY]=>[1000, 0, 714, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 121, :T_HASH]=>[1000, 0, 714, 0, 1, 0]} ``` After (this change): ``` {["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 213, :T_HASH]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 214, :T_OBJECT]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 214, :T_HASH]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 81, :T_OBJECT]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 304, :T_STRING]=>[1000, 0, 738, 0, 1, 0]} ```
* | | Make ActiveRecord `ConnectionPool.connections` thread-safe. (#36473)jeffdoering2019-06-133-1/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Make ActiveRecord `ConnectionPool.connections` thread-safe. ConnectionPool documentation is clear on the need to synchronize access to @connections but also states that public methods do not require synchronization. Existing code exposed @connections directly via attr_reader. The fix uses synchronize() to lock @connections then returns a copy to the caller using Array.dup(). Includes comments on the connections method that thread-safe access to the connections array does not imply thread-safety of accessing methods on the actual connections. Adds a test-case that modifies the pool using a supported method in one thread while a second thread accesses pool.connections. The test fails without this patch. Fixes #36465. * Update activerecord/test/cases/connection_pool_test.rb [jeffdoering + Rafael Mendonça França]
* | | Ensure to reset migration version after testing migrationRyuta Kamizono2019-06-131-2/+2
| | | | | | | | | | | | | | | | | | | | | "schema_migrations" table may be hard dropped before, so the reset migration version should be done in ensure block. https://buildkite.com/rails/rails/builds/61697#18d6f3ac-2257-4f4b-8efc-4010464c4d9a/999-1011
* | | Reset migration version before testing migrationRyuta Kamizono2019-06-131-0/+2
| | | | | | | | | | | | https://buildkite.com/rails/rails/builds/61695#373bb1a7-677f-49ec-95e7-a92467fefd60/1076-1084
* | | Avoid implicit rollback when testing migrationRyuta Kamizono2019-06-131-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | "schema_migrations" is hard dropped by some existing tests, so testing migration in using transactional tests may cause implicit creation and rollback "schema_migrations" table, it makes migration tests flaky. https://buildkite.com/rails/rails/builds/61692#42383249-30be-4508-b1fb-a7bb27600c8e/999-1010 https://buildkite.com/rails/rails/builds/61694#6e462ad3-41d8-4e26-95ce-728495b0ac64/999-1010
* | | Merge pull request #36472 from kamipo/empty_line_only_before_access_modifierRyuta Kamizono2019-06-13151-178/+1
|\ \ \ | | | | | | | | Enable `Layout/EmptyLinesAroundAccessModifier` cop
| * | | Enable `Layout/EmptyLinesAroundAccessModifier` copRyuta Kamizono2019-06-13151-178/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We sometimes say "✂️ newline after `private`" in a code review (e.g. https://github.com/rails/rails/pull/18546#discussion_r23188776, https://github.com/rails/rails/pull/34832#discussion_r244847195). Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style `EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059). That cop and enforced style will reduce the our code review cost.
* | | | Remove duplicated `table_exists?`Ryuta Kamizono2019-06-132-8/+0
| | | | | | | | | | | | | | | | | | | | | | | | `table_exists?` is already exist in `ModelSchema`. https://github.com/rails/rails/blob/5cab344494c340ea82a35b46efa06b94f0b7730b/activerecord/lib/active_record/model_schema.rb#L339-L341
generated by cgit v1.2.3 (git 2.25.1) at 2024-12-29 10:25:44 +0000