aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
Commit message (Collapse)AuthorAgeFilesLines
...
* | | | Merge pull request #36029 from kamipo/deprecate_where_notRyuta Kamizono2019-04-235-15/+118
|\ \ \ \ | | | | | | | | | | 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-195-15/+118
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `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.
* | | | | Avoid new string instance creation in `InsertAll#execute`Ryuta Kamizono2019-04-221-3/+3
| | | | |
* | | | | Don't table name qualify aggrigate column for virtual attributeRyuta Kamizono2019-04-221-4/+2
| | | | | | | | | | | | | | | | | | | | Related 0ee96d13de29680e148ccb8e5b68025f29fd091c.
* | | | | Merge pull request #36052 from kamipo/fast_idRyuta Kamizono2019-04-229-26/+30
|\ \ \ \ \ | | | | | | | | | | | | PERF: 20% faster pk attribute access
| * | | | | PERF: 20% faster pk attribute accessRyuta Kamizono2019-04-229-26/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 ```
* | | | | | Remove useless `set_value` / `get_value` helper methodsRyuta Kamizono2019-04-222-17/+10
|/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Those helper methods makes relation values access 15% slower. https://gist.github.com/kamipo/e64439f7a206e1c5b5c69d92d982828e Before (02b5b8cb): ``` Warming up -------------------------------------- #limit_value 237.074k i/100ms #limit_value = 1 222.052k i/100ms Calculating ------------------------------------- #limit_value 6.477M (± 2.9%) i/s - 32.479M in 5.019475s #limit_value = 1 5.297M (± 4.3%) i/s - 26.424M in 4.999933s ``` After (this change): ``` Warming up -------------------------------------- #limit_value 261.109k i/100ms #limit_value = 1 239.646k i/100ms Calculating ------------------------------------- #limit_value 7.412M (± 1.6%) i/s - 37.077M in 5.003345s #limit_value = 1 6.134M (± 1.0%) i/s - 30.675M in 5.000908s ```
* | | | | Remove never used `database_selector` class accessorRyuta Kamizono2019-04-221-1/+0
| | | | | | | | | | | | | | | | | | | | It was never used from the beginning.
* | | | | Avoid method call if `@transaction_state` is not finalizedRyuta Kamizono2019-04-217-14/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Method call in Ruby is a bit slow. This makes attribute access 10% faster by avoiding method call (`sync_with_transaction_state`). Before (96cf7e0e): ``` Warming up -------------------------------------- user.id 131.291k i/100ms user['id'] 91.786k i/100ms user.name 151.605k i/100ms user['name'] 92.664k i/100ms user.changed? 17.772k i/100ms user.saved_changes? 23.909k i/100ms Calculating ------------------------------------- user.id 1.988M (± 7.0%) i/s - 9.978M in 5.051474s user['id'] 1.155M (± 5.8%) i/s - 5.783M in 5.022672s user.name 2.450M (± 4.3%) i/s - 12.280M in 5.021234s user['name'] 1.263M (± 2.1%) i/s - 6.394M in 5.066638s user.changed? 175.070k (±13.3%) i/s - 853.056k in 5.011555s user.saved_changes? 259.114k (±11.8%) i/s - 1.267M in 5.001260s ``` After (this change): ``` Warming up -------------------------------------- user.id 137.625k i/100ms user['id'] 96.054k i/100ms user.name 156.379k i/100ms user['name'] 94.795k i/100ms user.changed? 18.172k i/100ms user.saved_changes? 24.337k i/100ms Calculating ------------------------------------- user.id 2.201M (± 0.5%) i/s - 11.010M in 5.002955s user['id'] 1.320M (± 1.0%) i/s - 6.628M in 5.021293s user.name 2.677M (± 1.6%) i/s - 13.449M in 5.024399s user['name'] 1.314M (± 1.8%) i/s - 6.636M in 5.051444s user.changed? 190.588k (±11.1%) i/s - 944.944k in 5.065848s user.saved_changes? 262.782k (±12.1%) i/s - 1.290M in 5.028080s ```
* | | | | Merge pull request #36041 from ↵Ryuta Kamizono2019-04-201-1/+1
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | abhaynikam/change-deprecation-for-dynamic-route-segment-to-6.1 Change the deprecation message for dynamic routes segment to 6.1
| * | | | | Change deprecation message for dangerous query method to be disallowed in ↵Abhay Nikam2019-04-201-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | Rails 6.1
* | | | | | Remove description for namespaced `db:migrate:up`eileencodes2019-04-191-1/+0
|/ / / / / | | | | | | | | | | | | | | | | | | | | This was accidentally left in, the standard `db:migrate:up` doesn't have a description so `db:migrate:up:namespace` shouldn't have one either.
* | | | | Handle up/down for multiple databaseseileencodes2019-04-192-0/+57
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This change adds the ability to run up/down for a database in a multi-db environment. If you have an app with a primary and animals database the following tasks will be generated: ``` VERSION=123 rake db:migrate:up:primary VERSION=123 rake db:migrate:up:primary VERSION=123 rake db:migrate:down:primary VERSION=123 rake db:migrate:up:animals ``` I didn't generate descriptions with them since we don't generate a description for a single database application. In addition to this change I've made it so if your application has multiple databases Rails will raise if you try to run `up` or `down` without a namespace. This is because we don't know which DB you want to run `up` or `down` against unless the app tells us, so it's safer to just block it and recommend using namespaced versions of up/down respectively. The output for the raise looks like: ``` You're using a multiple database application. To use `db:migrate:down` you must run the namespaced task with a VERSION. Available tasks are db:migrate:down:primary and db:migrate:down:animals. ```
* | | | | Revert "Deprecate `collection_cache_key` which is private API"Ryuta Kamizono2019-04-193-6/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* | | | | Merge pull request #35998 from itsWill/add_documentation_for_add_indexEileen M. Uchitelle2019-04-191-0/+20
|\ \ \ \ \ | | | | | | | | | | | | Document algorithm: concurrent option for PostgreSQL [ci skip]
| * | | | | Document algorithm: concurrent options for PostgreSQL [ci skip]Guilherme Mansur2019-04-161-0/+20
| | | | | |
* | | | | | Deprecate `collection_cache_key` which is private APIRyuta Kamizono2019-04-193-3/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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-192-2/+11
|\ \ \ \ \ \ | | | | | | | | | | | | | | Introduce Actionable Errors
| * | | | | | Make ActiveRecord::PendingMigrationError actionableGenadi Samokovarov2019-04-192-2/+10
| | | | | | |
| * | | | | | Introduce Actionable ErrorsGenadi Samokovarov2019-04-191-0/+1
| | |/ / / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Actionable errors let's you dispatch actions from Rails' error pages. This can help you save time if you have a clear action for the resolution of common development errors. The de-facto example are pending migrations. Every time pending migrations are found, a middleware raises an error. With actionable errors, you can run the migrations right from the error page. Other examples include Rails plugins that need to run a rake task to setup themselves. They can now raise actionable errors to run the setup straight from the error pages. Here is how to define an actionable error: ```ruby class PendingMigrationError < MigrationError #:nodoc: include ActiveSupport::ActionableError action "Run pending migrations" do ActiveRecord::Tasks::DatabaseTasks.migrate end end ``` To make an error actionable, include the `ActiveSupport::ActionableError` module and invoke the `action` class macro to define the action. An action needs a name and a procedure to execute. The name is shown as the name of a button on the error pages. Once clicked, it will invoke the given procedure.
* | | | | | 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-193-3/+16
| | | | | | | | | | | | | | | | | | | | | | | | This follows #35595.
* | | | | | Revert renamed `GeneratedAttributeMethods`Ryuta Kamizono2019-04-191-3/+3
|/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This partly reverts 8ca3c286a768038f6b391fd3bfbdfcc299876a1f. `.is_a?(GeneratedAttributeMethodsBuilder)` doesn't make sense to me, the module class's `new` creates just an instance of the module class itself.
* | | | | Ensure active record can load without Railtieseileencodes2019-04-182-7/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When I added the rake tasks for multiple databases I accidentally made it so you couldn't use Active Record without Railties. Since getting the database configuration depends on being able to load Railties (there is no other way to read and parse the yaml file and load it) I've decided that using multiple databases outside of Rails is a no-op. I've changed the code here to return if Rails isn't defined. Then I changed the other tasks to use `ActiveRecord::Tasks::DatabaseTasks.env` instead of `Rails.env`. The multi-db tasks can keep using `Rails.env` because they'll only be generated if we're using Rails and not just Active Record.
* | | | | Merge pull request #35987 from kamipo/fix_dirty_tracking_after_rollbackRyuta Kamizono2019-04-179-25/+92
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | Fix dirty tracking after rollback.
| * | | | | Fix dirty tracking after rollback.Ryuta Kamizono2019-04-169-25/+92
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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-164-15/+89
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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-164-5/+18
|\ \ \ \ \ \ | | | | | | | | | | | | | | Cache full MySQL version in schema cache
| * | | | | | Make changes per PR feedbackAli Ibrahim2019-04-122-9/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Remove AbstractMysqlAdapter::Version since full_version_string will always be set. * Remove nodoc on private methods because private methods are not exposed in the docs.
| * | | | | | Cache full MySQL version in schema cacheAli Ibrahim2019-04-114-6/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 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.
* | | | | | | Merge pull request #35985 from jhawthorn/lazy_backtrace_cleanKasper Timm Hansen2019-04-161-1/+1
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | Find query_source_location using lazy Enumerator
| * | | | | | | Find query_source_location using lazy EnumeratorJohn Hawthorn2019-04-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This way, we only need to filter the backtrace up to the first non-noise stack frame. This also updates noise to be able to deal with being passed a lazy enum. We don't need this anywhere, but it seemed better for this to be consistent.
* | | | | | | | Merge pull request #35989 from koic/bump_rubocop_to_0_67_2Ryuta Kamizono2019-04-161-2/+2
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | Bump RuboCop to 0.67.2
| * | | | | | | | 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 ```
* / | | | | | | Format a comment to not show up as code [ci skip]Sharang Dashputre2019-04-161-2/+2
|/ / / / / / /
* | | | | | | Add CHANGELOG entry for d1107f4d1e2573948d4941ac44511a0af6241f80Ryuta Kamizono2019-04-161-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | [ci skip]
* | | | | | | Don't refer `@transaction_state` directlyRyuta Kamizono2019-04-161-5/+5
|/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since 8180c39, remaining transaction state is cleared in `force_clear_transaction_record_state` to less work `sync_with_transaction_state`. But it caused a race condition that `@transaction_state` would be cleared by other threads if the state is finalized. To work as before, snapshot `@transaction_state` to local variable not to refer `@transaction_state` directly. Fixes #35983.
* | | | | | Merge pull request #35977 from prathamesh-sonpatki/rm-required-in-generatorsRafael França2019-04-151-1/+1
|\ \ \ \ \ \ | |_|/ / / / |/| | | | | Remove `required: true` from the model generator template
| * | | | | Remove `required: true` from the model generator templatePrathamesh Sonpatki2019-04-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `belongs_to` association have `required: true` by default https://github.com/rails/rails/pull/18937 onwards so we don't need it in the generator template. We still need the code for required in the command line generator as it adds `null: false` in the migration.
* | | | | | Merge pull request #35899 from ↵Eileen M. Uchitelle2019-04-152-17/+51
|\ \ \ \ \ \ | |/ / / / / |/| | | | | | | | | | | | | | | | | 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-082-17/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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-155-9/+50
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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-159-7/+182
|\ \ \ \ \ \ | | | | | | | | | | | | | | make change_column_comment and change_table_comment invertible
| * | | | | | make change_column_comment and change_table_comment invertibleYoshiyuki Kinjo2019-04-159-7/+182
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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`.
* | | | | | | Refactor `sync_with_transaction_state`Ryuta Kamizono2019-04-151-8/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | All transaction states (:committed, :fully_committed, :rolledback, :fully_rolledback) are mutually exclusive. And also, `force_clear_transaction_record_state` should clear `@transaction_state` as well.
* | | | | | | Remove useless `update_attributes_from_transaction_state` and ↵Ryuta Kamizono2019-04-141-14/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | `set_transaction_state`
* | | | | | | Don't expose `add_to_transaction`Ryuta Kamizono2019-04-144-18/+12
|/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `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-144-48/+43
|\ \ \ \ \ \ \ | |/ / / / / / |/| | | | | | use PostgreSQL's bulk_alter_table implementation