aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/test/cases/associations
Commit message (Collapse)AuthorAgeFilesLines
* Deduplicate joins valuesRyuta Kamizono2019-08-021-0/+10
| | | | | | | | | #36805 have one possible regression that failing deduplication if `joins_values` have complex order (e.g. `joins_values = [join_node_a, :comments, :tags, join_node_a]`). This fixes the deduplication to take it in the first phase before grouping.
* Merge pull request #36708 from ↵Kasper Timm Hansen2019-07-311-0/+18
|\ | | | | | | | | rails/has-one-polymorphic-touch-dont-cache-association-result-inside-create-transaction Polymorphic has_one touch: Don't cache association result inside crea…
| * Polymorphic has_one touch: Reset association cache result after create ↵Kasper Timm Hansen2019-07-311-0/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | transaction In case of a polymorphic association there's no automatic inverse_of to assign the inverse record. So to get the record there needs to be a query executed, however, if the query fires within the transaction that's trying to create the associated record, no record can be found. And worse, the nil result is cached on the association so after the transaction commits the record can't be found. That's what happens if touch is enabled on a polymorphic has_one association. Consider a Comment with a commentable association that needs to be touched. For `Comment.create(commentable: Post.new)`, the existing code essentially does `commentable.send(:comment)` within the create transaction for the comment and thus not finding the comment. Now we're purposefully clearing the cache in case we've tried accessing the association within the transaction and found no object. Before: ``` kaspth-imac 2.6.3 ~/code/rails/activerecord master *= ARCONN=postgresql bin/test test/cases/associations/has_one_associations_test.rb -n /commit/ Using postgresql Run options: -n /commit/ --seed 46022 D, [2019-07-19T03:30:37.864537 #96022] DEBUG -- : Chef Load (0.2ms) SELECT "chefs".* FROM "chefs" WHERE "chefs"."employable_id" = $1 AND "chefs"."employable_type" = $2 LIMIT $3 [["employable_id", 1], ["employable_type", "DrinkDesignerWithPolymorphicTouchChef"], ["LIMIT", 1]] D, [2019-07-19T03:30:37.865013 #96022] DEBUG -- : Chef Create (0.2ms) INSERT INTO "chefs" ("employable_id", "employable_type") VALUES ($1, $2) RETURNING "id" [["employable_id", 1], ["employable_type", "DrinkDesignerWithPolymorphicTouchChef"]] D, [2019-07-19T03:30:37.865201 #96022] DEBUG -- : TRANSACTION (0.1ms) RELEASE SAVEPOINT active_record_1 D, [2019-07-19T03:30:37.874136 #96022] DEBUG -- : TRANSACTION (0.1ms) ROLLBACK D, [2019-07-19T03:30:37.874323 #96022] DEBUG -- : TRANSACTION (0.1ms) ROLLBACK F Failure: HasOneAssociationsTest#test_polymorphic_has_one_with_touch_option_on_create_wont_cache_assocation_so_fetching_after_transaction_commit_works [/Users/kaspth/code/rails/activerecord/test/cases/associations/has_one_associations_test.rb:716]: --- expected +++ actual @@ -1 +1 @@ -#<Chef id: 1, employable_id: 1, employable_type: "DrinkDesignerWithPolymorphicTouchChef", department_id: nil, employable_list_type: nil, employable_list_id: nil> +nil ``` After: ``` kaspth-imac 2.6.3 ~/code/rails/activerecord master *= ARCONN=postgresql bin/test test/cases/associations/has_one_associations_test.rb -n /commit/ Using postgresql Run options: -n /commit/ --seed 46022 D, [2019-07-19T03:30:22.479387 #95973] DEBUG -- : Chef Create (0.3ms) INSERT INTO "chefs" ("employable_id", "employable_type") VALUES ($1, $2) RETURNING "id" [["employable_id", 1], ["employable_type", "DrinkDesignerWithPolymorphicTouchChef"]] D, [2019-07-19T03:30:22.479574 #95973] DEBUG -- : TRANSACTION (0.1ms) RELEASE SAVEPOINT active_record_1 D, [2019-07-19T03:30:22.482051 #95973] DEBUG -- : Chef Load (0.1ms) SELECT "chefs".* FROM "chefs" WHERE "chefs"."employable_id" = $1 AND "chefs"."employable_type" = $2 LIMIT $3 [["employable_id", 1], ["employable_type", "DrinkDesignerWithPolymorphicTouchChef"], ["LIMIT", 1]] D, [2019-07-19T03:30:22.482317 #95973] DEBUG -- : TRANSACTION (0.1ms) ROLLBACK D, [2019-07-19T03:30:22.482437 #95973] DEBUG -- : TRANSACTION (0.1ms) ROLLBACK . Finished in 0.088498s, 11.2997 runs/s, 22.5994 assertions/s. 1 runs, 2 assertions, 0 failures, 0 errors, 0 skips ``` Notice the select now fires after the commit.
* | Preserve user supplied joins order as much as possibleRyuta Kamizono2019-07-301-8/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, string joins are always applied as last joins part, and Arel join nodes are always applied as leading joins part (since #36304), it makes people struggled to preserve user supplied joins order. To mitigate this problem, preserve the order of string joins and Arel join nodes either before or after of association joins. Fixes #36761. Fixes #34328. Fixes #24281. Fixes #12953.
* | Use capture_sql helper method in testsst00122019-07-282-8/+2
| |
* | Use match? where we don't need MatchDataAkira Matsuda2019-07-272-2/+2
| | | | | | | | We're already running Performance/RegexpMatch cop, but it seems like the cop is not always =~ justice
* | Fix join middle table alias when using HABTMTakayuki Nakata2019-07-261-2/+9
|/ | | | | | | | In using HABTM, join middle table alias is combined with the associated models name without sort, while middle table name is combined with those models name with sort. Fixes #36742.
* Enable `Layout/EmptyLinesAroundAccessModifier` copRyuta Kamizono2019-06-135-4/+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.
* Allow column name with function (e.g. `length(title)`) as safe SQL stringRyuta Kamizono2019-06-101-4/+4
| | | | | | | | | | | | | | | | Currently, almost all "Dangerous query method" warnings are false alarm. As long as almost all the warnings are false alarm, developers think "Let's ignore the warnings by using `Arel.sql()`, it actually is false alarm in practice.", so I think we should effort to reduce false alarm in order to make the warnings valuable. This allows column name with function (e.g. `length(title)`) as safe SQL string, which is very common false alarm pattern, even in the our codebase. Related 6c82b6c99, 6607ecb2a, #36420. Fixes #32995.
* Merge pull request #36429 from bogdan/fix-preloading-duplicate-recordsRyuta Kamizono2019-06-071-0/+5
|\ | | | | Fix preloading on AR::Relation where records are duplicated by a join
| * Fix preloading on AR::Relation where records are duplicated by a joinBogdan Gusiev2019-06-061-0/+5
| |
* | Merge pull request #36426 from abhaynikam/bump-codeclimate-rubocop-versionRyuta Kamizono2019-06-061-2/+0
|\ \ | | | | | | Bump rubocop to 0.71
| * | Bump rubocop to 0.71Abhay Nikam2019-06-061-2/+0
| |/
* / Allow quoted identifier string as safe SQL stringRyuta Kamizono2019-06-061-1/+1
|/ | | | | | | | | | | | | Currently `posts.title` is regarded as a safe SQL string, but `"posts"."title"` (it is a result of `quote_table_name("posts.title")`) is regarded as an unsafe SQL string even though a result of `quote_table_name` should obviously be regarded as a safe SQL string, since the column name matcher doesn't respect quotation, it is a little annoying. This changes the column name matcher to allow quoted identifiers as safe SQL string, now all results of the `quote_table_name` are regarded as safe SQL string.
* Address intermittent CI failure due to non-determined sort orderRyuta Kamizono2019-05-301-6/+7
| | | | https://buildkite.com/rails/rails/builds/61384#ad441461-87d8-4bdc-a71f-61921fe2df2e/993-1004
* Address intermittent CI failure in cascaded_eager_loading_test.rbRyuta Kamizono2019-05-291-12/+12
| | | | https://buildkite.com/rails/rails/builds/61362#99165d42-172d-4ad5-bf72-b29d8cd44f3e/995-1006
* Address intermittent CI failure due to unfilled schema columns cacheRyuta Kamizono2019-05-291-3/+3
| | | | https://buildkite.com/rails/rails/builds/61358#a78ee50e-30b5-48a2-858f-63eba287d919/1290-1298
* Give up filling schema cache before `assert_no_queries`Ryuta Kamizono2019-05-224-68/+17
| | | | | | | | | | | | | | | | | This reverts commit a1ee4a9ff9d4a3cb255365310ead0dc7b739c6be. Even if a1ee4a9 is applied, CI is still flakiness. https://buildkite.com/rails/rails/builds/61252#2c090afa-aa84-4a2b-8b81-9f09219222c6/994-1005 https://buildkite.com/rails/rails/builds/61252#2e55bf83-1bde-44a2-a4f1-b5c3f6820fb4/929-938 Failing tests by whether schema cache is filled or not, it actually means that whether SCHEMA SQLs are executed or not is not target for the tests. So I've reverted commit a1ee4a9 which filling schema cache before `assert_no_queries`, and replace `assert_no_queries` to `assert_queries(0)`.
* Implicit through table joins should be appeared before user supplied joinsRyuta Kamizono2019-05-191-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | #36293 was an issue for through association with `joins` for a long time, but since #35864 through association with `left_joins` would also be affected by the issue. Implicit through table joins should be appeared before user supplied joins, otherwise loading through association with joins will cause a statement invalid error. Fixes #36293. ``` % ARCONN=postgresql bundle exec ruby -w -Itest test/cases/associations/has_many_through_associations_test .rb -n test_through_association_with_joins Using postgresql Run options: -n test_through_association_with_joins --seed 7116 # Running: E Error: HasManyThroughAssociationsTest#test_through_association_with_joins: ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: missing FROM-clause entry for table "posts" LINE 1: ... "comments_posts" ON "comments_posts"."post_id" = "posts"."i... ^ : SELECT "comments".* FROM "comments" INNER JOIN "comments" "comments_posts" ON "comments_posts"."post_id" = "posts"."id" INNER JOIN "posts" ON "comments"."post_id" = "posts"."id" WHERE "posts"."author_id" = $1 rails test test/cases/associations/has_many_through_associations_test.rb:61 Finished in 0.388657s, 2.5730 runs/s, 0.0000 assertions/s. 1 runs, 0 assertions, 0 failures, 1 errors, 0 skips ```
* Merge pull request #36284 from kamipo/fix_eager_loading_with_string_joinsRyuta Kamizono2019-05-151-0/+11
|\ | | | | Fix eager loading associations with string joins not to raise NoMethodError
| * Fix eager loading associations with string joins not to raise NoMethodErrorRyuta Kamizono2019-05-151-0/+11
| | | | | | | | Fixes #34456.
* | Don't track implicit `touch` mutationRyuta Kamizono2019-05-131-5/+15
|/ | | | | | | | | | | | | | | This partly reverts the effect of d1107f4d. d1107f4d makes `touch` tracks the mutation whether the `touch` is occurred by explicit or not. Existing apps expects that the previous changes tracks only the changes which is explicit action by users. I'd revert the implicit `touch` mutation tracking since I'd not like to break existing apps. Fixes #36219.
* Namespace association extension modules under the owner modelJean Boussier2019-05-021-2/+2
|
* Make scope arity check consistent (#36134)Rob Trame2019-05-011-0/+18
| | | | | | | | * Make scope arity check consistent * Add test for arity change [Rob Trame + Rafael Mendonça França]
* Fix merging left_joins to maintain its own `join_type` contextRyuta Kamizono2019-04-271-0/+4
| | | | | | | | | | | | | | | 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-241-6/+10
|\ | | | | Model error as object
| * Raise deprecation for calling `[:f] = 'b'` or `[:f] << 'b'`lulalala2019-03-311-6/+10
| | | | | | | | Revert some tests to ensure back compatibility
* | Merge pull request #35869 from ↵Ryuta Kamizono2019-04-251-1/+38
|\ \ | | | | | | | | | | | | | | | 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-251-1/+38
| | |
* | | Fix sliced IN clauses to be groupedRyuta Kamizono2019-04-241-4/+4
|/ / | | | | | | | | | | | | 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
| |
* | Fix `automatic_inverse_of` not to be disabled if extension block is givenRyuta Kamizono2019-04-123-10/+19
| | | | | | | | | | | | | | | | If an association has a scope, `automatic_inverse_of` is to be disabled. But extension block is obviously not a scope. It should not be regarded as a scope. Fixes #28806.
* | Merge pull request #28155 from lcreid/belongs_toRyuta Kamizono2019-04-101-0/+16
|\ \ | | | | | | | | | Fix "autosave: true" on belongs_to of join model causes invalid records to be saved
| * | Fix circular `autosave: true`Larry Reid2018-07-231-0/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use a variable local to the `save_collection_association` method in `activerecord/lib/active_record/autosave_association.rb`, instead of an instance variable. Prior to this PR, when there was a circular series of `autosave: true` associations, the callback for a `has_many` association was run while another instance of the same callback on the same association hadn't finished running. When control returned to the first instance of the callback, the instance variable had changed, and subsequent associated records weren't saved correctly. Specifically, the ID field for the `belongs_to` corresponding to the `has_many` was `nil`. Remove unnecessary test and comments. Fixes #28080.
* | | Association loading isn't to be affected by scoping consistentlyRyuta Kamizono2019-04-051-5/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Follow-up of 5c71000, #29834, and #30271. Currently, preloading and eager loading are not to be affected by scoping, with the exception of `unscoped`. But non eager loaded association access is still affected by scoping. Although this is a breaking change, the association loading will work consistently whether preloaded / eager loaded or not. Before: ```ruby Post.where("1=0").scoping do Comment.find(1).post # => nil Comment.preload(:post).find(1).post # => #<Post id: 1, ...> Comment.eager_load(:post).find(1).post # => #<Post id: 1, ...> end ``` After: ```ruby Post.where("1=0").scoping do Comment.find(1).post # => #<Post id: 1, ...> Comment.preload(:post).find(1).post # => #<Post id: 1, ...> Comment.eager_load(:post).find(1).post # => #<Post id: 1, ...> end ``` Fixes #34638. Fixes #35398.
* | | Stash `left_joins` into `joins` to deduplicate redundant LEFT JOINRyuta Kamizono2019-04-052-1/+14
| |/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Originally the `JoinDependency` has the deduplication for eager loading (LEFT JOIN). This re-uses that deduplication for `left_joins`. And also, This makes left join order into part of joins, i.e.: Before: ``` association joins -> stash joins (eager loading, etc) -> string joins -> left joins ``` After: ``` association joins -> stash joins (eager loading, left joins, etc) -> string joins ``` Now string joins are able to refer left joins. Fixes #34325. Fixes #34332. Fixes #34536.
* | Fix callbacks on has_many :through associations (#33249)Ryan Kerr2019-03-301-0/+18
| | | | | | | | | | | | | | | | | | | | | | When adding a child record via a has_many :through association, build_through_record would previously build the join record, and then assign the child record and source_type option to it. Because the before_add and after_add callbacks are called as part of build, however, this caused the callbacks to receive incomplete records, specifically without the other end of the has_many :through association. Collecting all attributes before building the join record ensures the callbacks receive the fully constructed record.
* | Merge pull request #35496 from bogdan/right-preloadingRyuta Kamizono2019-03-281-0/+9
|\ \ | | | | | | Fix preloader to never reset associations in case they are already loaded
| * | Fix preloader to never reset associations in case they are already loadedBogdan Gusiev2019-03-071-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch fixes the issue when association is preloaded with a custom preload scope which disposes the already preloaded target of the association by reseting it. When custom preload scope is used, the preloading is now performed into a separated Hash - #records_by_owner instead of the association. It removes the necessaty the reset the association after the preloading is complete so that reset of the preloaded association never happens. Preloading is still happening to the association when the preload scope is empty.
* | | Fix CI failure due to remaining tagging recordsRyuta Kamizono2019-03-261-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | `TRUNCATE TABLE posts` also resets `AUTO_INCREMENT`. If newly created a post, it is wrongly associated with remaining tagging records. To un-associate remaining tagging record, use `post.create_tagging!` instead. Fixes #35751.
* | | Use `assert_queries(0)` instead of `assert_no_queries` to ignore metadata ↵Yasuo Honda2019-03-261-1/+1
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | queries Fix #35665 ```ruby $ ARCONN=mysql2 bin/test test/cases/scoping/named_scoping_test.rb test/cases/tasks/database_tasks_test.rb test/cases/associations/cascaded_eager_loading_test.rb test/cases/associations/eager_singularization_test.rb -n "/^(?:NamedScopingTest#(?:test_many_should_not_fire_query_if_scope_loaded)|ActiveRecord::DatabaseTasksDumpSchemaCacheTest#(?:test_dump_schema_cache)|CascadedEagerLoadingTest#(?:test_eager_association_loading_with_has_many_sti_and_subclasses)|EagerSingularizationTest#(?:test_eager_no_extra_singularization_has_many_through_belongs_to))$/" --seed 16818 Using mysql2 Run options: -n "/^(?:NamedScopingTest#(?:test_many_should_not_fire_query_if_scope_loaded)|ActiveRecord::DatabaseTasksDumpSchemaCacheTest#(?:test_dump_schema_cache)|CascadedEagerLoadingTest#(?:test_eager_association_loading_with_has_many_sti_and_subclasses)|EagerSingularizationTest#(?:test_eager_no_extra_singularization_has_many_through_belongs_to))$/" --seed 16818 ...F Failure: CascadedEagerLoadingTest#test_eager_association_loading_with_has_many_sti_and_subclasses [/home/yahonda/git/rails/activerecord/test/cases/associations/cascaded_eager_loading_test.rb:124]: 1 instead of 0 queries were executed. Queries: SHOW FULL FIELDS FROM `topics`. Expected: 0 Actual: 1 bin/test test/cases/associations/cascaded_eager_loading_test.rb:119 Finished in 6.894609s, 0.5802 runs/s, 1.0153 assertions/s. 4 runs, 7 assertions, 1 failures, 0 errors, 0 skips $ ```
* | Remove unnecessary `current_adapter?(:OracleAdapter)` for index lengthYasuo Honda2019-03-031-6/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Follow up #35455, there are two more test cases unnecessary `if current_adapter?(:OracleAdapter)` ```ruby $ ARCONN=oracle bin/test test/cases/associations/eager_test.rb -n test_include_has_many_using_primary_key Using oracle Run options: -n test_include_has_many_using_primary_key --seed 62842 . Finished in 50.280024s, 0.0199 runs/s, 0.0398 assertions/s. 1 runs, 2 assertions, 0 failures, 0 errors, 0 skips $ ``` ``` $ ARCONN=oracle bin/test test/cases/migration/index_test.rb -n test_add_index Using oracle Run options: -n test_add_index --seed 52034 . Finished in 13.152620s, 0.0760 runs/s, 0.0000 assertions/s. 1 runs, 0 assertions, 0 failures, 0 errors, 0 skips $ ```
* | Fix test that was broken by adding a default scope to an existing modelRafael Mendonça França2019-02-261-2/+2
| |
* | Fix preload with nested associationsRafael Mendonça França2019-02-261-0/+9
| | | | | | | | | | | | When the middle association doesn't have any records and the inner association is not an empty scope the owner will be `nil` so we can't try to reset the inverse association.
* | Merge pull request #35247 from bogdan/fix-source-reflection-reset-codeRyuta Kamizono2019-02-202-3/+11
|\ \ | | | | | | Fix reset of the source association when through association is loaded
| * | Fix reset of the source association when through association is loadedBogdan Gusiev2019-02-202-3/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | The special case happens when through association has a custom scope that is applied to the source association when loading. In this case, the soucre association would need to be reset after main association is loaded. See tests. The special case exists when a through association has
* | | Fix eager loading polymorphic association with mixed table conditionsRyuta Kamizono2019-02-181-1/+13
| | | | | | | | | | | | | | | | | | This fixes a bug that the `foreign_key` and the `foreign_type` are separated as different table conditions if a polymorphic association has a scope that joins another tables.
* | | Revert "Merge pull request #35127 from bogdan/counter-cache-loading"Ryuta Kamizono2019-02-131-36/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit eec3e28a1abf75676dcee58308ee5721bb53c325, reversing changes made to 5588fb4802328a2183f4a55c36d6703ee435f85c. Reason: Marking as loaded without actual loading is too greedy optimization. See more context #35239. Closes #35239. [Edouard CHIN & Ryuta Kamizono]
* | | Fix `pluck` and `select` with custom attributesRyuta Kamizono2019-02-131-2/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | Currently custom attributes are always qualified by the table name in the generated SQL wrongly even if the table doesn't have the named column, it would cause an invalid SQL error. Custom attributes should only be qualified if the table has the same named column.
* | | Fix random CI failure due to non-deterministic sorting orderRyuta Kamizono2019-02-101-3/+3
| | | | | | | | | | | | https://travis-ci.org/rails/rails/jobs/491045821#L1528-L1531