| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|\
| |
| | |
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
|
|
|
|
| |
Fixes #34456.
|
|
|
|
|
|
|
|
| |
* Make scope arity check consistent
* Add test for arity change
[Rob Trame + Rafael Mendonça França]
|
|
|
|
|
|
|
| |
Follow up of #35838.
And also this refactors `in_clause_length` handling is entirely
integrated in Arel visitor.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
$
```
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
|
|
|
| |
The PR#31819 changed `#preloaders_on` and added some test,
then #33938 reverted changes that were added to the method in #31819.
Since changes in the method were reverted and as mentioned in the
comment https://github.com/rails/rails/pull/31819#discussion_r221847481
that titles of the tests added in #31819 don't reflect implementation I
think we can remove those test for now.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since #33844, eager loading/preloading with too many and/or too large
ids won't be broken by pre-checking whether the value is constructable
or not.
But the pre-checking caused the type to be evaluated at relation build
time instead of at the query execution time, that is breaking an
expectation for some apps.
I've made the pre-cheking lazy as much as possible, that is no longer
happend at relation build time.
|
|
|
|
|
|
|
|
|
| |
`test_update_does_not_run_sql_if_record_has_not_changed` would pass
without #18501 since `assert_queries` ignores BEGIN/COMMIT unless
`ignore_none: true` is given.
Since #32647, empty BEGIN/COMMIT is ommited. So we no longer need to use
`assert_queries(0)` to ignore BEGIN/COMMIT in the queries.
|
|
|
|
|
|
| |
If you pass a hash-like object to preload associations (for example ActionController::Parameters)
preloader will fail with the ArgumentError.
This change allows passing objects that may be converted to a Hash or String into a preloader
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since 213796f, bind params are used for IN clause if enabled prepared
statements.
Unfortunately, most adapter modules have a limitation for # of bind
params (mysql2 65535, pg 65535, sqlite3 250000). So if eager loading
large number of records at once, that query couldn't be sent to the
database.
Since eager loading/preloading queries are auto-generated by Active
Record itself, so it should be worked regardless of large number of
records like as before.
Fixes #33702.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Six Mocha calls prove quite resistant to Minitestification. For example,
if we replace
```
ActiveRecord::Associations::HasManyAssociation
.any_instance
.expects(:reader)
.never
```
with `assert_not_called`, Minitest wisely raises
```
NameError: undefined method `reader' for class `ActiveRecord::Associations::HasManyAssociation'
```
as `:reader` comes from a deeply embedded abstract class,
`ActiveRecord::Associations::CollectionAssociation`.
This patch tackles this difficulty by adding
`ActiveSupport::Testing::MethodCallAsserts#assert_called_on_instance_of`
which injects a stubbed method into `klass`, and verifies the number of
times it is called, similar to `assert_called`. It also adds a convenience
method, `assert_not_called_on_instance_of`, mirroring
`assert_not_called`.
It uses the new method_call_assertions to replace the remaining Mocha
calls in `ActiveRecord` tests.
[utilum + bogdanvlviv + kspath]
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, the seen object cache is shared if join nodes have the same
target class. But it is a wrong assumption, we can't share the seen
object cache between different join nodes (e.g. `:readonly_account` and
`:accounts` have the same target class `Account`, but the instances
have the different state `readonly`).
Fixes #26805.
Closes #27737.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently, column aliases which is used for eager loading are calculated
before constructing all table aliases in FROM clause.
`JoinDependency#join_constraints` constructs table aliases for `joins`
first, and then always re-constructs table aliases for eager loading.
If both `joins` and eager loading are given a same table association,
the re-construction would cause the discrepancy between column aliases
and table aliases.
To avoid the discrepancy, the column aliases should be calculated after
all table aliases are constructed.
Fixes #30603.
|
| |
|
|
|
|
|
|
|
|
|
| |
When a belongs to association's target is set, its foreign key is now
updated to match the new target. This is the correct behaviour when a
new record is assigned, but not when the existing record is preloaded.
As long as we mark the association as loaded, we can skip setting the
target when the record is missing and avoid clobbering the foreign key.
|
|
|
|
|
|
|
|
|
|
| |
This reverts ignoring polymorphic error introduced at 02da8ae.
What the ignoring want to solve was caused by force eager loading
regardless of whether it is necessary, but it has been fixed by #29043.
The ignoring is now only causing a mismatch of `exists?` behavior with
`to_a`, `count`, etc. It should behave consistently.
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Currently reflections keeps the order when first added even if when
redefining association. As a result of the order, redefining through
association which use newly added association will raise
`HasManyThroughOrderError`. We need to redefine reflection order as well
when redefining association.
Fixes #31068.
|
|
|
|
| |
Follow up of #31432.
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
`exists?`
This test covers the case of 02da8aea.
Previously `exists?` was always eager-loading the includes values. But
now it is eager-loaded only when necessary since 07a611e0.
So the case of the eager-loading had not covered in the test.
|
|
|
|
| |
Because this is not a test case.
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If `reflection_scope.where_clause` is not empty, `through_scope` should
be joined the source association. But if the through association doesn't
have explicit `:source`, `options[:source]` will be nil and
`scope.includes_values` will also be empty. It should use
`source_reflection.name` rather than `options[:source]`.
Fixed by a26cff3c1235c61cd0135bed4ef63d7be452b458.
Fixes #11078.
Fixes #26129.
Closes #14312.
Closes #29155.
Closes #29841.
|
|
|
|
|
|
|
| |
This regression was caused by caa178c1. The block for
`set_inverse_instance` should also be passed to join dependency.
Fixes #30402.
|
| |
|
|
|
|
| |
Fixes #28324.
|
| |
|
|
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| |
|
|
|
|
| |
These `assert_nothing_raised` are covered by following assertions.
|
| |
|
|
|
|
| |
Fixes #29025.
|
|
|
|
|
|
|
| |
`assert_raise` does not check error message. However, in some tests,
it seems like expecting error message checking with `assert_raise`.
Instead of specifying an error message in `assert_raise`, modify to use
another assert to check the error message.
|
|
|
|
|
|
| |
These are followups for 307065f959f2b34bdad16487bae906eb3bfeaf28,
but TBH I'm personally not very much confortable with this style.
Maybe we could override assert_equal in our test_helper not to warn?
|
| |
|
| |
|
|
|
|
|
|
| |
assert [1, 3].includes?(2) fails with unhelpful "Asserting failed" message
assert_includes [1, 3], 2 fails with "Expected [1, 3] to include 2" which makes it easier to debug and more obvious what went wrong
|
|\
| |
| |
| | |
Preserve readonly flag only for readonly association
|
| |
| |
| |
| | |
Fixes #24093
|
| |
| |
| |
| |
| |
| |
| |
| | |
Style/SpaceBeforeBlockBraces
Style/SpaceInsideBlockBraces
Style/SpaceInsideHashLiteralBraces
Fix all violations in the repository.
|