aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/lib/active_record/associations
Commit message (Collapse)AuthorAgeFilesLines
...
* | | | Joined tables in association scope doesn't use the same aliases with the ↵Ryuta Kamizono2017-10-093-13/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | parent relation's aliases Building association scope in join dependency should respect the parent relation's aliases to avoid using the same alias name more than once. Fixes #30681.
* | | | Decouple building `AliasTracker` from `JoinDependency`Ryuta Kamizono2017-10-083-14/+7
| | | | | | | | | | | | | | | | | | | | This is preparation to respect parent relation's alias tracking for fixing #30681.
* | | | Move duplicated code to `delete_or_destroy` in `CollectionAssociation`Ryuta Kamizono2017-10-061-4/+2
| | | |
* | | | Ensure `AliasTracker` respects a custom table nameRyuta Kamizono2017-09-302-2/+2
| | | |
* | | | PERF: Partially recover some performance when preloading.Guo Xiang Tan2017-09-261-9/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Benchmark Script: ``` require 'active_record' require 'benchmark/ips' ActiveRecord::Base.establish_connection(ENV.fetch('DATABASE_URL')) ActiveRecord::Migration.verbose = false ActiveRecord::Schema.define do create_table :users, force: true do |t| t.string :name, :email t.integer :topic_id t.timestamps null: false end create_table :topics, force: true do |t| t.string :title t.timestamps null: false end end attributes = { name: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.', email: 'foobar@email.com' } class Topic < ActiveRecord::Base has_many :users end class User < ActiveRecord::Base belongs_to :topic end 100.times do User.create!(attributes) end users = User.first(50) Topic.create!(title: 'This is a topic', users: users) Benchmark.ips do |x| x.config(time: 10, warmup: 5) x.report("preload") do User.includes(:topic).all.to_a end end ``` Before: ``` Calculating ------------------------------------- preload 40.000 i/100ms ------------------------------------------------- preload 407.962 (± 1.5%) i/s - 4.080k ``` After: ``` alculating ------------------------------------- preload 43.000 i/100ms ------------------------------------------------- preload 427.567 (± 1.6%) i/s - 4.300k ```
* | | | Remove unused delegation to `reflection.options` in `Preloader::Association`Ryuta Kamizono2017-09-182-4/+1
| | | |
* | | | The name of the key on the associated record is abstracted as ↵Ryuta Kamizono2017-09-184-14/+5
| | | | | | | | | | | | | | | | `reflection.join_primary_key`
* | | | The name of the key on the owner is abstracted as `reflection.join_foreign_key`Ryuta Kamizono2017-09-184-17/+5
| | | |
* | | | Extract `associate_records_to_owner` to refactor `Preloader::Association`Ryuta Kamizono2017-09-183-20/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since we have `Preloader#preload`, `Preloader::Association#preload` is a little confusing. And also, since the `preload` method is an abstract method, it is hard to read where `associated_records_by_owner` is called. This refactors `Preloader::Association` to ease to read where `associated_records_by_owner` is called.
* | | | Remove the code that swapping `scope` and `options`Ryuta Kamizono2017-09-181-5/+0
| | | | | | | | | | | | | | | | `options` is never assigned to `scope` as long as using splat hash.
* | | | Early return if `records.empty?` in `Preloader#preload`Ryuta Kamizono2017-09-181-3/+3
| | | |
* | | | Don't pass `reflection_scope` to `preload_scope` if `reflection.scope` isn't ↵Ryuta Kamizono2017-09-181-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | given Related 2b5f5cdd7c1d95716de6a206b6d09ccbb006dc17. If `reflection.scope` isn't given, `reflection_scope` is always empty scope. It is unnecessary to merge it.
* | | | Return `through_scope` only if the scope is not empty scopeRyuta Kamizono2017-09-181-4/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Related 2b5f5cdd7c1d95716de6a206b6d09ccbb006dc17. If `through_scope` is empty scope, it is unnecessary to merge it. And also, comparing relations is a little expensive (will cause `build_arel`). It is enough to use `empty_scope?` to determine whether empty scope.
* | | | Remove useless condition in `reset_association`Ryuta Kamizono2017-09-181-2/+1
| | | | | | | | | | | | | | | | `through_scope` is not empty scope if `options[:source_type]` is given.
* | | | PERF: Incorrect memoization in ↵Guo Xiang Tan2017-09-111-1/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `ActiveRecord::Associations::Preloader::Association`. ``` require 'active_record' require 'benchmark/ips' ActiveRecord::Base.establish_connection(ENV.fetch('DATABASE_URL')) ActiveRecord::Migration.verbose = false ActiveRecord::Schema.define do create_table :users, force: true do |t| t.string :name, :email t.integer :topic_id t.timestamps null: false end create_table :topics, force: true do |t| t.string :title t.timestamps null: false end end attributes = { name: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.', email: 'foobar@email.com' } class Topic < ActiveRecord::Base has_many :users end class User < ActiveRecord::Base belongs_to :topic end 100.times do User.create!(attributes) end users = User.first(50) 100.times do Topic.create!(title: 'This is a topic', users: users) end Benchmark.ips do |x| x.config(time: 10, warmup: 5) x.report("preload") do User.includes(:topic).all.to_a end end ``` ``` Calculating ------------------------------------- preload 25.000 i/100ms ------------------------------------------------- preload 251.772 (± 1.2%) i/s - 2.525k ``` ``` Calculating ------------------------------------- preload 26.000 i/100ms ------------------------------------------------- preload 270.392 (± 1.1%) i/s - 2.704k ```
* | | | Remove unnecessary `join_type` in `AssociationScope`Ryuta Kamizono2017-09-081-5/+1
| | | | | | | | | | | | | | | | | | | | | | | | This method was moved from `JoinHelper` in 0fddc3c1, but it is only used for `table.create_join` in the internal and `Nodes::InnerJoin` is default join klass. So it is not needed to pass it explicitly.
* | | | Don't pass `table` to `last_chain_scope` and `next_chain_scope`Ryuta Kamizono2017-09-071-17/+15
| | | | | | | | | | | | | | | | | | | | | | | | Because `table` is part of `reflection`, don't need to pass it explicitly. And also, naming `alias_name` to `table` is a little confusing. `aliased_table` is preferable than `alias_name`.
* | | | `has_many :through` with unscope should affect to through scopeRyuta Kamizono2017-09-071-21/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The order of scope evaluation should be from through scope to the association's own scope. Otherwise the association's scope cannot affect to through scope. Fixes #13677. Closes #28449.
* | | | Don't pass unneeded `reflection` to `add_constraints`Ryuta Kamizono2017-09-051-3/+3
| | | | | | | | | | | | | | | | Because `refl.scope` is the same meaning with `chain_head.scope`.
* | | | Assigning `values` is only necessary when `reflection_scope.where_clause` is ↵Ryuta Kamizono2017-09-051-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | not empty Because `reflection_scope.values` will create extra new hash.
* | | | Don't merge `reflection_scope` if `reflection.scope` isn't givenRyuta Kamizono2017-09-051-1/+1
| | | | | | | | | | | | | | | | | | | | If `reflection.scope` isn't given, `reflection_scope` is always `klass.unscoped`. it is unnecessary to merge it.
* | | | Scope in associations should treat nil as `all`Ryuta Kamizono2017-09-044-5/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Defined scope treats nil as `all`, but scope in associations isn't so. If the result of the scope is nil, most features on associations will be broken. It should treat nil as `all` like defined scope. Fixes #20823.
* | | | Fix preloading through association with custom scopeRyuta Kamizono2017-09-021-1/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If `reflection_scope.where_clause` is not empty, `through_scope` should be joined the source association. But if `values[:references]` in `reflection_scope` is empty, the source association will not be joined. It should use `source_reflection.table_name` in that case. Fixes #22535. Closes #28763.
* | | | `values[:includes]` in `reflection_scope` is not compatible with `through_scope`Ryuta Kamizono2017-09-021-1/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Without this fix, preloading `:comments_with_include` will cause the following error: ``` % bundle exec ruby -w -Itest test/cases/associations/eager_test.rb -n test_eager_with_has_many_through_join_model_with_include Using sqlite3 Run options: -n test_eager_with_has_many_through_join_model_with_include --seed 1502 E Error: EagerAssociationTest#test_eager_with_has_many_through_join_model_with_include: ActiveRecord::AssociationNotFoundError: Association named 'post' was not found on Post; perhaps you misspelled it? ```
* | | | Don't call `scope.eager_loading?` when `reflection_scope.where_clause` is emptyRyuta Kamizono2017-09-021-10/+10
| | | | | | | | | | | | | | | | | | | | If `reflection_scope.where_clause` is empty, `scope` isn't changed. So `scope.eager_loading?` is always false.
* | | | Merge pull request #30403 from yhirano55/fix_broken_doc_for_active_recordRyuta Kamizono2017-08-267-7/+7
|\ \ \ \ | | | | | | | | | | Fix broken doc for Active Record [ci skip]
| * | | | Fix broken doc for Active Record [ci skip]Yoshiyuki Hirano2017-08-257-7/+7
| | | | |
* | | | | Should work inverse association when eager loadingRyuta Kamizono2017-08-251-3/+3
|/ / / / | | | | | | | | | | | | | | | | | | | | | | | | This regression was caused by caa178c1. The block for `set_inverse_instance` should also be passed to join dependency. Fixes #30402.
* | | | Through scope should not be affected by scopingRyuta Kamizono2017-08-152-12/+2
| | | | | | | | | | | | | | | | | | | | | | | | Follow up of #29834. Fixes #30266.
* | | | Merge pull request #27609 from kamipo/fix_association_primary_keyRafael França2017-08-141-4/+7
|\ \ \ \ | | | | | | | | | | Fix `reflection.association_primary_key` for `has_many` association
| * | | | Fix `reflection.association_primary_key` for `has_many` associationsRyuta Kamizono2017-08-131-4/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It is incorrect to treat `options[:primary_key]` as `association_primary_key` if `has_many` associations because the `:primary_key` means the column on the owner record, not on the association record. It will break `ids_reader` and `ids_writer`. ```ruby people(:david).essay_ids # => ActiveRecord::StatementInvalid: Mysql2::Error: Unknown column 'essays.first_name' in 'field list': SELECT `essays`.first_name FROM `essays` WHERE `essays`.`writer_id` = 'David' ``` Fixes #14439.
* | | | | Delegate to `Enumerable#find` for `CollectionProxy`Ryuta Kamizono2017-08-132-17/+14
|/ / / / | | | | | | | | | | | | | | | | Since `Relation` includes `Enumerable`, it is enough to use `super` simply.
* | | | Merge pull request #29720 from gaurish/ar_find_error_message_improvementRafael França2017-08-111-1/+3
|\ \ \ \ | | | | | | | | | | Return Not found Ids in ActiveRecord::NotFound
| * | | | Return Not found Ids in ActiveRecord::NotFoundGaurish Sharma2017-07-291-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | This builds on top of 15e2da656f41af0124f7577858536f3b65462ad5. now it also returns exact Ids which were not found which will be debugging simple.
* | | | | Specify `table.name` only when `scope.table` and `table` are different (#29058)Ryuta Kamizono2017-08-111-4/+12
| | | | | | | | | | | | | | | Fixes #29045.
* | | | | Passing `klass` to `StatementCache.new`Ryuta Kamizono2017-08-042-12/+8
|/ / / / | | | | | | | | | | | | | | | | | | | | Actually `StatementCache#execute` is always passed the same klass that the owner klass of the connection when the statement cache is created. So passing `klass` to `StatementCache.new` will make more DRY.
* | | | Remove single element array preprocessRyuta Kamizono2017-07-261-1/+1
| | | | | | | | | | | | | | | | | | | | Since 213796f, array predicate handler supports making binds, so the preprocess is no longer needed.
* | | | Remove useless `JoinInformation`Ryuta Kamizono2017-07-252-5/+3
| | | | | | | | | | | | | | | | | | | | Since 213796f removed `binds`, `JoinInformation` only contain `joins`. So it is enough to return `joins` simply.
* | | | Merge pull request #29765 from lugray/fix_counter_cacheRafael França2017-07-241-3/+1
|\ \ \ \ | | | | | | | | | | Fix `counter_cache` double increment
| * | | | Add test for fixed `counter_cache` double incrementLisa Ugray2017-07-191-3/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When an `after_create` callback did `update_attributes` on a record with multiple `belongs_to` associations with counter caches, even numbered associations would have their counters double-incremented. Fixes to `ActiveModel::Dirty` in 020abad fixed this. This adds regression tests for this bug fixed incidentally in the other commit, which also removed the need for the workaround using @_after_create_counter_called.
* | | | | Refactor Active Record to let Arel manage bind paramsSean Griffin2017-07-242-5/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A common source of bugs and code bloat within Active Record has been the need for us to maintain the list of bind values separately from the AST they're associated with. This makes any sort of AST manipulation incredibly difficult, as any time we want to potentially insert or remove an AST node, we need to traverse the entire tree to find where the associated bind parameters are. With this change, the bind parameters now live on the AST directly. Active Record does not need to know or care about them until the final AST traversal for SQL construction. Rather than returning just the SQL, the Arel collector will now return both the SQL and the bind parameters. At this point the connection adapter will have all the values that it had before. A bit of this code is janky and something I'd like to refactor later. In particular, I don't like how we're handling associations in the predicate builder, the special casing of `StatementCache::Substitute` in `QueryAttribute`, or generally how we're handling bind value replacement in the statement cache when prepared statements are disabled. This also mostly reverts #26378, as it moved all the code into a location that I wanted to delete. /cc @metaskills @yahonda, this change will affect the adapters Fixes #29766. Fixes #29804. Fixes #26541. Close #28539. Close #24769. Close #26468. Close #26202. There are probably other issues/PRs that can be closed because of this commit, but that's all I could find on the first few pages.
* | | | | Merge pull request #29855 from lugray/has_one_destroyed_by_associationRafael França2017-07-211-0/+2
|\ \ \ \ \ | | | | | | | | | | | | Match destroyed_by_association for has_one to has_many
| * | | | | Match destroyed_by_association for has_one to has_manyLisa Ugray2017-07-211-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a has_many association is destroyed by `dependent: destroy`, destroyed_by_association is set to the reflection, and this can be checked in callbacks. This matches that behaviour for has_one associations.
* | | | | | Use frozen-string-literal in ActiveRecordKir Shatrov2017-07-1935-0/+70
|/ / / / /
* | | | | Merge pull request #29033 from kamipo/make_preload_query_to_prepared_statementsSean Griffin2017-07-181-1/+1
|\ \ \ \ \ | |/ / / / |/| | | | Make preload query to preparable
| * | | | Make preload query to preparableRyuta Kamizono2017-07-071-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently preload query cannot be prepared statements even if `prepared_statements: true` due to array handler in predicate builder doesn't support making bind params. This makes preload query to preparable by don't passing array value if possible.
* | | | | Fix unscoping `default_scope` for `Preloader`Ryuta Kamizono2017-07-191-1/+11
| | | | |
* | | | | Fix `JoinDependency` with using a custom tableRyuta Kamizono2017-07-182-6/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Without this fix, `JoinDependency` doesn't use a custom table alias: ``` % ARCONN=sqlite3 be ruby -w -Itest test/cases/relations_test.rb -n test_using_a_custom_table_with_joins_affects_the_wheres Using sqlite3 Run options: -n test_using_a_custom_table_with_joins_affects_the_wheres --seed 14531 E Error:RelationTest#test_using_a_custom_table_with_joins_affects_the_wheres: ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: posts.author_id: SELECT "omg_posts".* FROM "posts" "omg_posts" INNER JOIN "authors" ON "authors"."id" = "posts"."author_id" WHERE "omg_posts"."title" = ? LIMIT ? ```
* | | | | Don't cache `scope_for_create`Ryuta Kamizono2017-07-162-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I investigated where `scope_for_create` is reused in tests with the following code: ```diff --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -590,6 +590,10 @@ def where_values_hash(relation_table_name = table_name) end def scope_for_create + if defined?(@scope_for_create) && @scope_for_create + puts caller + puts "defined" + end @scope_for_create ||= where_values_hash.merge!(create_with_value.stringify_keys) end ``` It was hit only `test_scope_for_create_is_cached`. This means that `scope_for_create` will not be reused in normal use cases. So we can remove caching `scope_for_create` to respect changing `where_clause` and `create_with_value`.
* | | | | Fix `create_with` using both string and symbolRyuta Kamizono2017-07-163-9/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is related with #27680. Since `where_values_hash` keys constructed by `where` are string, so we need `stringify_keys` to `create_with_value` before merging it.