aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/test
Commit message (Collapse)AuthorAgeFilesLines
* Remove ignored_sql from SQLCounter by adding "TRANSACTION" to log nameYasuo Honda2019-05-082-11/+3
| | | | | | | | | | This commit adds "TRANSACTION" to savepoint and commit, rollback statements because none of savepoint statements were removed by #36153 since they are not "SCHEMA" statements. Although, only savepoint statements can be labeled as "TRANSACTION" I think all of transaction related method should add this label. Follow up #36153
* Should attempt `committed!`/`rolledback!` to all enrolled records in the ↵Ryuta Kamizono2019-05-071-5/+51
| | | | | | | | | | | | | | | | | | | | | | transaction Currently, `committed!`/`rolledback!` will only be attempted for the first enrolled record in the transaction, that will cause some problematic behaviors. The first one problem, `clear_transaction_record_state` won't be called even if the transaction is finalized except the first enrolled record. This means that de-duplicated records in the transaction won't refer latest state (e.g. won't happen rolling back record state). The second one problem, the enrolled order is not always the same as the order in which the actions actually happened, the first enrolled record may succeed no actions (e.g. `destroy` has already succeeded on another record during `before_destroy`), it will lose to fire any transactional callbacks. To avoid both problems, we should attempt `committed!`/`rolledback!` to all enrolled records in the transaction.
* Remove redundant `test_too_many_binds`Yasuo Honda2019-05-031-20/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | with `ActiveRecord::BindParameterTest#test_too_many_binds` sqlite adapter has its own `bind_params_length`, `ActiveRecord::BindParameterTest#test_too_many_binds` respects it. * Modified `ActiveRecord::BindParameterTest#test_too_many_binds` to show `bind_params_length` value ``` $ git diff diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index 85685d1d00..83cd07f1d7 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -108,6 +108,7 @@ def test_statement_cache_with_sql_string_literal def test_too_many_binds bind_params_length = @connection.send(:bind_params_length) + p bind_params_length topics = Topic.where(id: (1 .. bind_params_length).to_a << 2**63) assert_equal Topic.count, topics.count $ ``` * Executed modified `ActiveRecord::BindParameterTest#test_too_many_binds` ``` $ bin/test test/cases/bind_parameter_test.rb -n test_too_many_binds Using sqlite3 Run options: -n test_too_many_binds --seed 47321 999 . Finished in 0.075249s, 13.2892 runs/s, 26.5784 assertions/s. 1 runs, 2 assertions, 0 failures, 0 errors, 0 skips $ ```
* Namespace association extension modules under the owner modelJean Boussier2019-05-022-10/+10
|
* Make scope arity check consistent (#36134)Rob Trame2019-05-012-0/+19
| | | | | | | | * Make scope arity check consistent * Add test for arity change [Rob Trame + Rafael Mendonça França]
* Remove database specific sql statements from SQLCounterYasuo Honda2019-05-013-19/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Every database executes different type of sql statement to get metadata then `ActiveRecord::TestCase` ignores these database specific sql statements to make `assert_queries` or `assert_no_queries` work consistently. Connection adapter already labels these statement by setting "SCHEMA" argument, this pull request makes use of "SCHEMA" argument to ignore metadata queries. Here are the details of these changes: * PostgresqlConnectionTest Each of PostgresqlConnectionTest modified just executes corresponding methods https://github.com/rails/rails/blob/fef174f5c524edacbcad846d68400e7fe114a15a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb#L182-L195 ```ruby # Returns the current database encoding format. def encoding query_value("SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = current_database()", "SCHEMA") end # Returns the current database collation. def collation query_value("SELECT datcollate FROM pg_database WHERE datname = current_database()", "SCHEMA") end # Returns the current database ctype. def ctype query_value("SELECT datctype FROM pg_database WHERE datname = current_database()", "SCHEMA") end ``` * BulkAlterTableMigrationsTest mysql2 adapter executes `SHOW KEYS FROM ...` to see if there is an index already created as below. I think the main concerns of these tests are how each database adapter creates or drops indexes then ignoring `SHOW KEYS FROM` statement makes sense. https://github.com/rails/rails/blob/fef174f5c524edacbcad846d68400e7fe114a15a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb#L11 ```ruby execute_and_free("SHOW KEYS FROM #{quote_table_name(table_name)}", "SCHEMA") do |result| ``` * Temporary change not included in this commit to show which statements executed ```diff $ git diff diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 8e8ed494d9..df05f9bd16 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -854,7 +854,7 @@ def test_adding_indexes classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] expected_query_count = { - "Mysql2Adapter" => 3, # Adding an index fires a query every time to check if an index already exists or not + "Mysql2Adapter" => 1, # Adding an index fires a query every time to check if an index already exists or not "PostgreSQLAdapter" => 2, }.fetch(classname) { raise "need an expected query count for #{classname}" @@ -886,7 +886,7 @@ def test_removing_index classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] expected_query_count = { - "Mysql2Adapter" => 3, # Adding an index fires a query every time to check if an index already exists or not + "Mysql2Adapter" => 1, # Adding an index fires a query every time to check if an index already exists or not "PostgreSQLAdapter" => 2, }.fetch(classname) { raise "need an expected query count for #{classname}" $ ``` * Executed these modified tests ```ruby $ ARCONN=mysql2 bin/test test/cases/migration_test.rb -n /index/ Using mysql2 Run options: -n /index/ --seed 8462 F Failure: BulkAlterTableMigrationsTest#test_adding_indexes [/home/yahonda/git/rails/activerecord/test/cases/migration_test.rb:863]: 3 instead of 1 queries were executed. Queries: SHOW KEYS FROM `delete_me` SHOW KEYS FROM `delete_me` ALTER TABLE `delete_me` ADD UNIQUE INDEX `awesome_username_index` (`username`), ADD INDEX `index_delete_me_on_name_and_age` (`name`, `age`). Expected: 1 Actual: 3 bin/test test/cases/migration_test.rb:848 F Failure: BulkAlterTableMigrationsTest#test_removing_index [/home/yahonda/git/rails/activerecord/test/cases/migration_test.rb:895]: 3 instead of 1 queries were executed. Queries: SHOW KEYS FROM `delete_me` SHOW KEYS FROM `delete_me` ALTER TABLE `delete_me` DROP INDEX `index_delete_me_on_name`, ADD UNIQUE INDEX `new_name_index` (`name`). Expected: 1 Actual: 3 bin/test test/cases/migration_test.rb:879 .. Finished in 0.379245s, 10.5473 runs/s, 7.9105 assertions/s. 4 runs, 3 assertions, 2 failures, 0 errors, 0 skips $ ``` * ActiveRecord::ConnectionAdapters::Savepoints Left `self.ignored_sql` to ignore savepoint related statements because these SQL statements are not related "SCHEMA" ``` self.ignored_sql = [/^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/] ``` https://github.com/rails/rails/blob/fef174f5c524edacbcad846d68400e7fe114a15a/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb#L10-L20 ```ruby def create_savepoint(name = current_savepoint_name) execute("SAVEPOINT #{name}") end def exec_rollback_to_savepoint(name = current_savepoint_name) execute("ROLLBACK TO SAVEPOINT #{name}") end def release_savepoint(name = current_savepoint_name) execute("RELEASE SAVEPOINT #{name}") end ```
* Should take the record's state of first action in the transactionRyuta Kamizono2019-04-301-0/+2
| | | | | | | | | | | | | | | | If the same id's records are saved and/or destroyed in the transaction, commit callbackes will only run for the first enrolled record. https://github.com/rails/rails/blob/a023e2180093ebc517a642aaf21f3c7241c67657/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb#L115-L119 The regression #36132 is caused due to #35920 changed the enrollment order that the first action's record will be enrolled to last in the transaction. We could not change the the enrollment order as long as someone depends on the enrollment order. Fixes #36132.
* Fix merging left_joins to maintain its own `join_type` contextRyuta Kamizono2019-04-272-0/+5
| | | | | | | | | | | | | | | This fixes a regression for #35864. Usually, stashed joins (mainly eager loading) are performed as LEFT JOINs. But the case of merging joins/left_joins of different class, that (stashed) joins are performed as the same `join_type` as the parent context for now. Since #35864, both (joins/left_joins) stashed joins might be contained in `joins_values`, so each stashed joins should maintain its own `join_type` context. Fixes #36103.
* Merge pull request #32313 from lulalala/model_error_as_objectRafael França2019-04-244-19/+25
|\ | | | | Model error as object
| * Assert deprecationlulalala2019-03-311-2/+2
| |
| * Raise deprecation for calling `[:f] = 'b'` or `[:f] << 'b'`lulalala2019-03-312-12/+16
| | | | | | | | Revert some tests to ensure back compatibility
| * Fix spec as generate_message is no longer called during validationlulalala2019-03-311-0/+2
| |
| * Fix misalignment caused by SHA eebb9ddf9ba559a510975c486fe59a4edc9da97dlulalala2019-03-311-5/+5
| |
* | Merge pull request #35869 from ↵Ryuta Kamizono2019-04-253-2/+43
|\ \ | | | | | | | | | | | | | | | abhaynikam/35866-add-touch-option-for-has-one-association Adds missing touch option to has_one association
| * | Adds touch option to has_one associationAbhay Nikam2019-04-253-2/+43
| | |
* | | Avoid redundant `attribute_alias?` before `attribute_alias`Ryuta Kamizono2019-04-241-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we want to get alias resolved attribute finally, we can use `attribute_alias` directly. For that purpose, avoiding redundant `attribute_alias?` makes alias attribute access 40% faster. https://gist.github.com/kamipo/e427f080a27b46f50bc508fae3612a0e Before (2c0729d8): ``` Warming up -------------------------------------- user['id'] 102.668k i/100ms user['new_id'] 80.660k i/100ms user['name'] 99.368k i/100ms user['new_name'] 81.626k i/100ms Calculating ------------------------------------- user['id'] 1.431M (± 4.0%) i/s - 7.187M in 5.031985s user['new_id'] 1.042M (± 4.2%) i/s - 5.243M in 5.039858s user['name'] 1.406M (± 5.6%) i/s - 7.055M in 5.036743s user['new_name'] 1.074M (± 3.6%) i/s - 5.387M in 5.024152s ``` After (this change): ``` Warming up -------------------------------------- user['id'] 109.775k i/100ms user['new_id'] 103.303k i/100ms user['name'] 105.988k i/100ms user['new_name'] 99.618k i/100ms Calculating ------------------------------------- user['id'] 1.520M (± 6.7%) i/s - 7.574M in 5.011496s user['new_id'] 1.485M (± 6.2%) i/s - 7.438M in 5.036252s user['name'] 1.538M (± 5.4%) i/s - 7.737M in 5.049765s user['new_name'] 1.516M (± 4.6%) i/s - 7.571M in 5.007293s ```
* | | Fix sliced IN clauses to be groupedRyuta Kamizono2019-04-246-46/+25
|/ / | | | | | | | | | | | | Follow up of #35838. And also this refactors `in_clause_length` handling is entirely integrated in Arel visitor.
* | Make association builder methods privateRyuta Kamizono2019-04-241-1/+1
| |
* | Merge pull request #36029 from kamipo/deprecate_where_notRyuta Kamizono2019-04-232-6/+51
|\ \ | | | | | | Deprecate `where.not` working as NOR and will be changed to NAND in Rails 6.1
| * | Deprecate `where.not` working as NOR and will be changed to NAND in Rails 6.1Ryuta Kamizono2019-04-192-6/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `where.not` with polymorphic association is partly fixed incidentally at 213796f (refer #33493, #26207, #17010, #16983, #14161), and I've added test case e9ba12f to avoid lose that fix accidentally in the future. In Rails 5.2, `where.not(polymorphic: object)` works as expected as NAND, but `where.not(polymorphic_type: object.class.polymorphic_name, polymorphic_id: object.id)` still unexpectedly works as NOR. To will make `where.not` working desiredly as NAND in Rails 6.1, this deprecates `where.not` working as NOR. If people want to continue NOR conditions, we'd encourage to them to `where.not` each conditions manually. ```ruby all = [treasures(:diamond), treasures(:sapphire), cars(:honda), treasures(:sapphire)] assert_equal all, PriceEstimate.all.map(&:estimate_of) ``` In Rails 6.0: ```ruby sapphire = treasures(:sapphire) nor = all.reject { |e| e.estimate_of_type == sapphire.class.polymorphic_name }.reject { |e| e.estimate_of_id == sapphire.id } assert_equal [cars(:honda)], nor without_sapphire = PriceEstimate.where.not( estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id ) assert_equal nor, without_sapphire.map(&:estimate_of) ``` In Rails 6.1: ```ruby sapphire = treasures(:sapphire) nand = all - [sapphire] assert_equal [treasures(:diamond), cars(:honda)], nand without_sapphire = PriceEstimate.where.not( estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id ) assert_equal nand, without_sapphire.map(&:estimate_of) ``` Resolves #31209.
* | | PERF: 20% faster pk attribute accessRyuta Kamizono2019-04-221-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I've realized that `user.id` is 20% slower than `user.name` in the benchmark (https://github.com/rails/rails/pull/35987#issuecomment-483882480). The reason that performance difference is that `self.class.primary_key` method call is a bit slow. Avoiding that method call will make almost attribute access faster and `user.id` will be completely the same performance with `user.name`. Before (02b5b8cb): ``` Warming up -------------------------------------- user.id 140.535k i/100ms user['id'] 96.549k i/100ms user.name 158.110k i/100ms user['name'] 94.507k i/100ms user.changed? 19.003k i/100ms user.saved_changes? 25.404k i/100ms Calculating ------------------------------------- user.id 2.231M (± 0.9%) i/s - 11.243M in 5.040066s user['id'] 1.310M (± 1.3%) i/s - 6.565M in 5.012607s user.name 2.683M (± 1.2%) i/s - 13.439M in 5.009392s user['name'] 1.322M (± 0.9%) i/s - 6.615M in 5.003239s user.changed? 201.999k (±10.9%) i/s - 1.007M in 5.091195s user.saved_changes? 258.214k (±17.1%) i/s - 1.245M in 5.007421s ``` After (this change): ``` Warming up -------------------------------------- user.id 158.364k i/100ms user['id'] 106.412k i/100ms user.name 158.644k i/100ms user['name'] 107.518k i/100ms user.changed? 19.082k i/100ms user.saved_changes? 24.886k i/100ms Calculating ------------------------------------- user.id 2.768M (± 1.1%) i/s - 13.936M in 5.034957s user['id'] 1.507M (± 2.1%) i/s - 7.555M in 5.017211s user.name 2.727M (± 1.5%) i/s - 13.643M in 5.004766s user['name'] 1.521M (± 1.3%) i/s - 7.634M in 5.018321s user.changed? 200.865k (±11.1%) i/s - 992.264k in 5.044868s user.saved_changes? 269.652k (±10.5%) i/s - 1.344M in 5.077972s ```
* | | Revert "Deprecate `collection_cache_key` which is private API"Ryuta Kamizono2019-04-191-3/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit f656bb301a43fe441af0039e4fafe40a7faa62f8. Reason: Test in Action View expects the `collection_cache_key` working... https://github.com/rails/rails/blob/ff6b713f5e729859995f204093ad3f8e08f39ea8/actionview/test/activerecord/relation_cache_test.rb#L21 https://github.com/rails/rails/blob/ff6b713f5e729859995f204093ad3f8e08f39ea8/actionview/test/fixtures/project.rb#L6 https://buildkite.com/rails/rails/builds/60609#d19181fb-fe80-4d1e-891c-1109b540fb4b/981-1009
* | | Deprecate `collection_cache_key` which is private APIRyuta Kamizono2019-04-191-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | The `collection_cache_key` is private API for a long time, but I've maintained it in #35848 since it is mentioned in the doc (https://github.com/rails/rails/pull/35848#discussion_r272011475). The doc has removed at 1da9a7e4, so there is no longer a reason to maintain that private API.
* | | Merge pull request #34788 from gsamokovarov/actionable-errorsKasper Timm Hansen2019-04-191-2/+4
|\ \ \ | | | | | | | | Introduce Actionable Errors
| * | | Make ActiveRecord::PendingMigrationError actionableGenadi Samokovarov2019-04-191-2/+4
| |/ /
* | | Fix const `Post` leakage in `test/cases/base_test.rb`Ryuta Kamizono2019-04-191-0/+2
| | | | | | | | | | | | https://buildkite.com/rails/rails/builds/60601#81daf2ee-9259-42e5-983d-c57d66499b76/928-939
* | | Give GeneratedRelationMethods module a nameRyuta Kamizono2019-04-192-2/+12
|/ / | | | | | | This follows #35595.
* | Merge pull request #35987 from kamipo/fix_dirty_tracking_after_rollbackRyuta Kamizono2019-04-171-0/+59
|\ \ | | | | | | | | | Fix dirty tracking after rollback.
| * | Fix dirty tracking after rollback.Ryuta Kamizono2019-04-161-0/+59
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently the rollback only restores primary key value, `new_record?`, `destroyed?`, and `frozen?`. Since the `save` clears current dirty attribute states, retrying save after rollback will causes no change saved if partial writes is enabled (by default). This makes `remember_transaction_record_state` remembers original values then restores dirty attribute states after rollback. Fixes #15018. Fixes #30167. Fixes #33868. Fixes #33443. Closes #33444. Closes #34504.
* | | Add collection cache versioningLachlan Sylvester2019-04-161-0/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Cache versioning enables the same cache key to be reused when the object being cached changes by moving the volatile part of the cache key out of the cache key and into a version that is embedded in the cache entry. This is already occurring when the object being cached is an `ActiveRecord::Base`, but when caching an `ActiveRecord::Relation` we are currently still putting the volatile information (max updated at and count) as part of the cache key. This PR moves the volatile part of the relations `cache_key` into the `cache_version` to support recycling cache keys for `ActiveRecord::Relation`s.
* | | Merge pull request #35946 from alimi/cache-full-mysql-database-versionKasper Timm Hansen2019-04-161-0/+4
|\ \ \ | | | | | | | | Cache full MySQL version in schema cache
| * | | Cache full MySQL version in schema cacheAli Ibrahim2019-04-111-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * The database version is cached in all the adapters, but this didn't include the full MySQL version. Anything that uses the full MySQL version would need to query the database to get that data even if they're using the schema cache. * Now the full MySQL version will be cached in the schema cache via the Version object.
* | | | Bump RuboCop to 0.67.2Koichi ITO2019-04-161-2/+2
| |/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Performance cops will be extracted from RuboCop to RuboCop Performance when next RuboCop 0.68 will be released. https://github.com/rubocop-hq/rubocop/issues/5977 RuboCop 0.67 is its transition period. Since rails/rails repository uses Performance cops, This PR added rubocop-performance gem to Gemfile. And this PR fixes some offenses using the following auto-correct. ```console % bundle exec rubocop -a Offenses: activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:212:26: C: [Corrected] Layout/SpaceAroundOperators: Operator = > should be surrounded by a single space. "primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" } ^^ activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:239:26: C: [Corrected] Layout/SpaceAroundOperators: Operator => should be surrounded by a single space. "primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" } ^^ actionview/test/template/resolver_shared_tests.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true. module ResolverSharedTests ^ actionview/test/template/resolver_shared_tests.rb:10:33: C: [Corrected] Layout/SpaceAroundEqualsInParameterDefault: Surrounding space missing in default value assignment. def with_file(filename, source="File at #{filename}") ^ actionview/test/template/resolver_shared_tests.rb:106:5: C: [Corrected] Rails/RefuteMethods: Prefer assert_not_same over refute_same. refute_same a, b ^^^^^^^^^^^ 2760 files inspected, 5 offenses detected, 5 offenses corrected ```
* | | Merge pull request #35899 from ↵Eileen M. Uchitelle2019-04-151-7/+34
|\ \ \ | | | | | | | | | | | | | | | | eileencodes/fix-connection-when-handler-doesnt-exist Ensure a handler is set when using `connected_to`
| * | | Ensure a handler is set when using `connected_to`eileencodes2019-04-081-7/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | After looking at #35800 there is definitely an issue in the `connected_to` method although it's generally behaving. Here are the details: 1) I added a default connection role - writing - to the connection handler lookup. I did this because otherwise if you did this: ``` connected_to(databse: :development) ``` And development wasn't a pre-established role it would create a new handler and connect using that. I don't think this is right so I've updated it to pick up the default (:writing) unless otherwise specified. To set a handler when using the database version pass a hash like you would to `connects_to`: ``` connected_to(database: { readonly_slow: :development }) ``` This will connect the `development` database to the `readonly_slow` handler/role. 2) I updated the tests to match this behavior that we expect. 3) I updated the documentation to clarify that using `connected_to` with a `database` key will establish a new connection every time. This is exactly how `establish_connection` behaves and I feel this is correct. If you want to only establish a connection once you should do that in the model with `connects_to` and then swap on the role instead of on the database hash/key. 4) In regards to #35800 this fixes the case where you pass a symbol to the db and not a hash. But it doesn't fix a case where you may pass an unknown handler to an abstract class that's not connected. This is tricky because technical AbstractFoo doesn't have any connections except for through ApplicationRecord, so in the case of the application that was shared we should only be swapping connections on ActiveRecord::Base because there are no other actual connections - AbstractFoo isn't needed since it's not establishing a new connection. If we need AbstractFoo to connect to a new handler we should establish that connection with the handler in AbstractFoo before trying to shard there.
* | | | Fix dirty tracking for `touch`Ryuta Kamizono2019-04-152-4/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Before this fix, `touch` only clears dirty tracking for touched attributes, doesn't track saved (touched) changes. This fixes that tracks saved changes and carry over remaining changes. Fixes #33429. Closes #34306.
* | | | Merge pull request #35970 from yskkin/reversible_commentRyuta Kamizono2019-04-153-0/+122
|\ \ \ \ | | | | | | | | | | make change_column_comment and change_table_comment invertible
| * | | | make change_column_comment and change_table_comment invertibleYoshiyuki Kinjo2019-04-153-0/+122
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We can revert migrations using `change_column_comment` or `change_table_comment` at current master. However, results are not what we expect: comments are remained in new status. This change tells previous comment to these methods in a way like `change_column_default`.
* | | | | Don't expose `add_to_transaction`Ryuta Kamizono2019-04-142-4/+4
|/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `add_to_transaction` was added at da840d1, but it should not be called by except internal, since `remember_transaction_record_state` should be called only once before saving. And also, currently `add_to_transaction` doesn't always add the record to transaction since da8de91, that is the reason hard to use that even in internal. Even if `add_to_transaction` ensure to add the record to transaction, that is an internal concern, people don't need to explicitly call `add_to_transaction`.
* | | | Merge pull request #35966 from shioyama/define_validator_on_topic_subclassRyuta Kamizono2019-04-141-5/+7
|\ \ \ \ | | | | | | | | | | Add validation to subclass in tests to avoid polluting parent class
| * | | | Add validation to subclass in tests to avoid polluting parent classChris Salzberg2019-04-131-5/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | These two tests currently both define acceptance validators on the same class, Topic. This means that in either one test or the other, there are not one but *two* instances of the LazilyDefineAttributes module builder in the class' ancestors, which can result in unpredictable results. Subclassing Topic in each test avoids conflicts.
* | | | | Merge pull request #35958 from yskkin/bulk_change_tableRyuta Kamizono2019-04-141-7/+17
|\ \ \ \ \ | |/ / / / |/| | | | use PostgreSQL's bulk_alter_table implementation
| * | | | use PostgreSQL's bulk_alter_table implementationYoshiyuki Kinjo2019-04-131-7/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Running this migration on mysql at current master fails because `add_references_for_alter` is missing. ``` change_table :users, bulk: true do |t| t.references :article end ``` This is also true for postgresql adapter, but its `bulk_alter_table` implementation can fallback in such case. postgresql's implementation is desirable to prevent unknown failure like this.
* | | | | Auto-correct `Style/StringLiterals` cop offencesRyuta Kamizono2019-04-131-4/+4
| | | | | | | | | | | | | | | | | | | | Somehow Code Climate is not working as expected for now?
* | | | | Fix `presicion` -> `precision`yuuji.yaginuma2019-04-132-6/+6
| | | | | | | | | | | | | | | | | | | | This fix is necessary to test precision's default correctly.
* | | | | Fix test flakyness due to `test_truncate_tables`Guilherme Mansur2019-04-121-0/+16
|/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `Truncate Tables posts` will also reset the `AUTOINCREMENT` this causes a situation where if a test suite that uses the `taggings` fixtures runs and subsequently the `test_truncate_tables` run, newly created posts would reference the `tagging` in the database. This commit resest the state of the posts table after the `connection.truncate` call in the `test_truncate_tables`, as well as all other tests that call `trucate` This ensures the associations and db state remain consistent after the tests. Fixes: https://github.com/rails/rails/issues/35941
* | | | Merge pull request #35918 from ↵Ryuta Kamizono2019-04-121-6/+8
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | kamipo/lazy_sync_with_transaction_state_on_destroy Lazy sync with transaction state on destroy
| * | | | Lazy sync with transaction state on destroyRyuta Kamizono2019-04-101-6/+8
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 58410b3d566e6b93c7b71c0eec0fc11ec906b68e. If we have any implicit commit/rollback callbacks, it is necessary to add record to transaction explicitly like `:touch_deferred_attributes`. https://github.com/rails/rails/blob/5f261d04d6f857d49c75124df809adfbd6cd5b5e/activerecord/lib/active_record/touch_later.rb#L9 https://github.com/rails/rails/blob/5f261d04d6f857d49c75124df809adfbd6cd5b5e/activerecord/lib/active_record/touch_later.rb#L25 But I can't find any other implicit commit/rollback callbacks in our code base at least now. I think the `self.class.connection.add_transaction_record(self)` line doesn't cover any behavior.
* | | | Merge pull request #35920 from ↵Ryuta Kamizono2019-04-122-4/+26
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | kamipo/dont_call_commit_callbacks_for_invalid_record Don't call after_commit callbacks despite a record isn't saved
| * | | | Don't call after_commit callbacks despite a record isn't savedRyuta Kamizono2019-04-122-4/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Regardless of a record isn't saved (e.g. validation is failed), `after_commit` / `after_rollback` callbacks are invoked for now. To fix the issue, this adds a record to the current transaction only when a record is actually saved. Fixes #29747. Closes #29833.
generated by cgit v1.2.3 (git 2.25.1) at 2024-06-29 16:03:21 +0000