| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|\
| |
| | |
Fix preloading on AR::Relation where records are duplicated by a join
|
| | |
|
|\ \
| | |
| | | |
Bump rubocop to 0.71
|
| |/ |
|
|/
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
| |
https://buildkite.com/rails/rails/builds/61384#ad441461-87d8-4bdc-a71f-61921fe2df2e/993-1004
|
|
|
|
| |
https://buildkite.com/rails/rails/builds/61362#99165d42-172d-4ad5-bf72-b29d8cd44f3e/995-1006
|
|
|
|
| |
https://buildkite.com/rails/rails/builds/61358#a78ee50e-30b5-48a2-858f-63eba287d919/1290-1298
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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)`.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
#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
```
|
|\
| |
| | |
Fix eager loading associations with string joins not to raise NoMethodError
|
| |
| |
| |
| | |
Fixes #34456.
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
|
| |
* Make scope arity check consistent
* Add test for arity change
[Rob Trame + Rafael Mendonça França]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|\
| |
| | |
Model error as object
|
| |
| |
| |
| | |
Revert some tests to ensure back compatibility
|
|\ \
| | |
| | |
| | |
| | |
| | | |
abhaynikam/35866-add-touch-option-for-has-one-association
Adds missing touch option to has_one association
|
| | | |
|
|/ /
| |
| |
| |
| |
| |
| | |
Follow up of #35838.
And also this refactors `in_clause_length` handling is entirely
integrated in Arel visitor.
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| | |
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.
|
|\ \
| | |
| | |
| | | |
Fix "autosave: true" on belongs_to of join model causes invalid records to be saved
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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.
|
| |/
|/|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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.
|
|\ \
| | |
| | | |
Fix preloader to never reset associations in case they are already loaded
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
`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.
|
|/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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
$
```
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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
$
```
|
| | |
|
| |
| |
| |
| |
| |
| | |
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.
|
|\ \
| | |
| | | |
Fix reset of the source association when through association is loaded
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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
|
| | |
| | |
| | |
| | |
| | |
| | | |
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.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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]
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
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.
|
| | |
| | |
| | |
| | | |
https://travis-ci.org/rails/rails/jobs/491045821#L1528-L1531
|
|\ \ \
| |/ /
|/| | |
Bugfix has_many association #size when ids reader is cached and assoc…
|
| | |
| | |
| | |
| | | |
is changed
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
order by for metadata queries
Also, `test_has_one_does_not_use_order_by` should not check metadata queries.
Fixes #35098
|
|/ /
| |
| |
| | |
Formerly it was returning arguments (`records` array).
|
|\ \
| | |
| | | |
Bugfix association loading behavior when counter cache is zero
|
| | | |
|
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This reverts commit ed1eda271c7ac82ecb7bd94b6fa1b0093e648a3e, reversing
changes made to 3d2caab7dc92a13d4dd369678d5b4ce659df8e52.
Reason: 7c3da6e0030aa080fcb89af58b094ed50d861a44
|