| Commit message (Collapse) | Author | Age | Files | Lines |
|\
| |
| |
| | |
Ensure that `delete_all` on collection proxy returns affected count
|
| |
| |
| |
| |
| |
| |
| | |
Unlike the `Relation#delete_all`, `delete_all` on collection proxy
doesn't return affected count. Since the `CollectionProxy` is a subclass
of the `Relation`, this inconsistency is probably not intended, so it
should return the count consistently.
|
|/
|
|
|
| |
Reset scope after delete on collection association to clear stale
offsets of removed records.
|
|
|
|
| |
Follow up ba4e68f577efc76f351d30a2914e29942b97830e.
|
| |
|
|
|
|
| |
https://travis-ci.org/rails/rails/jobs/459534536#L1280
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There was a bug in the handling of duplicates when
assigning (replacing) associated records, which made the result
dependent on whether a given record was associated already before
being assigned anew. E.g.
post.people = [person, person]
post.people.count
# => 2
while
post.people = [person]
post.people = [person, person]
post.people.count
# => 1
This change adds a test to provoke the former incorrect behavior, and
fixes it.
Cause of the bug was the handling of record collections as sets, and
using `-` (difference) and `&` (union) operations on them
indiscriminately. This temporary conversion to sets would eliminate
duplicates.
The fix is to decorate record collections for these operations, and
only for the `has_many :through` case. It is done by counting
occurrences, and use the record together with the occurrence number as
element, in order to make them work well in sets. Given
a, b = *Person.all
then the collection used for finding the difference or union of
records would be internally changed from
[a, b, a]
to
[[a, 1], [b, 1], [a, 2]]
for these operations. So a first occurrence and a second occurrence
would be distinguishable, which is all that is necessary for this
task.
Fixes #33942.
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Starting in Rails 5.0.0 and still present in Rails 5.2.1, `singular_ids`
got memoized and didn't reload after more items were added to the
relation.
Although 19c8071 happens to fix the issue, it only adds tests for
`has_many` relations while this bug only affected
`has_and_belongs_to_many` relations.
This commit adds a regression test to ensure it never happens again with
`habtm` relations.
Ensures #34179 never gets reproduced.
|
|
|
|
|
|
|
|
|
| |
`developers.name desc` was added at d59f3a7, but any test case isn't
failed even if the `developers.name desc` is removed since all tested
developers are consistently ordered on both `name` and `id`.
I changed one developers creation ordering to ensure to test that
`project.developers` is ordered by `developers.name desc`.
|
|\
| |
| | |
Generate delegation methods to named scope in the definition time
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The delegation methods to named scope are defined when `method_missing`
is invoked on the relation.
Since #29301, the receiver in the named scope is changed to the relation
like others (e.g. `default_scope`, etc) for consistency.
Most named scopes would be delegated from relation by `method_missing`,
since we don't allow scopes to be defined which conflict with instance
methods on `Relation` (#31179). But if a named scope is defined with the
same name as any method on the `superclass` (e.g. `Kernel.open`), the
`method_missing` on the relation is not invoked.
To address the issue, make the delegation methods to named scope is
generated in the definition time.
Fixes #34098.
|
|\ \
| |/
|/|
| |
| | |
christophemaximin/fix-activerecord-clearing-of-query-cache
Fix inconsistent behavior by clearing QueryCache when reloading associations
|
| | |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
flakiness
Follow up 45be690f8e6db019aac6198ba49d608a2e14824b.
Somehow calling `define_attribute_methods` in `build`/`new` sometimes
causes the `table_exists?` query.
To address CI flakiness due to `assert_no_queries` failure, ensure
`define_attribute_methods` before `assert_no_queries`.
|
| | |
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since #31575, `set_inverse_instance` replaces the foreign key by the
current owner immediately to make it happen when a record is added to
collection association.
But `set_inverse_instance` is not only called when a record is added,
but also when a record is loaded from queries. And also, that loaded
records are not always associated records for some reason (using `or`,
`unscope`, `rewhere`, etc).
It is hard to distinguish whether or not we should invoke
`set_inverse_instance`, but at least we should avoid the undesired
side-effect which was brought from #31575.
Fixes #34108.
|
|
|
|
| |
Follow up 811be477786455d144819a5e9fbb7f9f54b8da69.
|
|
|
|
|
|
|
|
|
| |
`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.
|
|
|
|
| |
cherry-picked from https://github.com/rails/rails/pull/33763
|
|
|
|
| |
missing records
|
|
|
|
| |
Since counter cache handles touch option too.
|
|\
| |
| | |
If association is a hash-like object preloading fails
|
| |
| |
| |
| |
| |
| | |
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
|
| |
| |
| |
| |
| |
| |
| | |
`association.increment_counters` and `association.decrement_counters`
works regardless of parent target is loaded or not.
Related 52e11e462f6114a4d12225c639c5f501f0ffec7a.
|
| |
| |
| |
| |
| |
| |
| |
| | |
This reverts commit 376ffe0ea2e59dc51461122210729c05a10fb443.
Since 38fae1f, `association.increment_counters` is called without
inflated parent target if inverse_of is disabled.
In that case, that commit would cause extra queries to inflate parent.
|
|/
|
|
| |
Fixes #19550.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This is a 4th attempt to make counter cache transactional completely.
Past attempts: #9236, #14849, #23357.
All existing counter cache issues (increment/decrement twice, lost
increment) are caused due to updating counter cache on the outside of
the record saving transaction by assigning belongs_to record, even
though assigning that doesn't cause the record saving.
We have the `@_after_replace_counter_called` guard condition to mitigate
double increment/decrement issues, but we can't completely prevent that
inconsistency as long as updating counter cache on the outside of the
transaction, since saving the record is not always happened after that.
We already have handling counter cache after create/update/destroy,
https://github.com/rails/rails/blob/1b90f614b1b3d06b7f02a8b9ea6cd84f15d58643/activerecord/lib/active_record/counter_cache.rb#L162-L189
https://github.com/rails/rails/blob/1b90f614b1b3d06b7f02a8b9ea6cd84f15d58643/activerecord/lib/active_record/associations/builder/belongs_to.rb#L33-L59
so just removing assigning logic on the belongs_to association makes
counter cache transactional completely.
Closes #14849.
Closes #23357.
Closes #31493.
Closes #31494.
Closes #32372.
Closes #33113.
Closes #33117
Closes #33129.
Closes #33458.
|
|
|
|
|
|
|
|
|
| |
`topic` and `reply` belongs_to associations on `SillyReply` are defined
with the same `foreign_key` (`parent_id`) and `counter_cache`
(`replies_count`) columns.
This would cause unintentional side-effect (e.g. saving `SillyReply`
object would cause double increment `replies_count`), so it is better to
avoid that side-effect.
|
|
|
|
| |
same record
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
| |
Without this, `inverse_associations_test.rb` breaks when running in isolation.
https://travis-ci.org/rails/rails/jobs/422266840#L1894-L1899
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Previously ActiveRecord couldn't find inverse associations if they were plural, which is a pretty standard use case. This commit changes the behavior to first attempt to find the singular version, then attempt to find the plural version. That makes it work and find plural associations as in the example below:
```
class Post
has_many :comments
end
class Comment
belongs_to :post
end
```
Previously the `:post` association reflection would only attempt to find a `comment` inverse, as opposed to both a `comment` and `comments` inverse.
|
| |
|
|
|
|
|
|
| |
There is test in has_one to test enum, but there is no for has_many.
[Rich Chen]
|
|\
| |
| | |
Stop using Mocha
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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]
|
| |
| |
| |
| |
| |
| | |
without define the enum in class “SpecialBook”, any string status will be casted to integer 0. Then the test have no meaning.
[Rich Chen]
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since fe9547b6, numericality validator would parse raw value only when a
value came from user to work type casting to a value from database.
But that was caused a regression that the validator would work against
getter value instead of parsed raw value, a getter is sometimes
customized by people. #33550
There we never guarantees that the value before type cast was going to
the used in this validation (actually here is only place that getter
value might not be used), but we should not change the behavior unless
there is some particular reason.
The purpose of fe9547b6 is to work type casting to a value from
database. We could achieve the purpose by using `read_attribute`,
without using getter value.
Fixes #33550.
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
| |
hash condition
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
| |
It mark the association as loaded and this can cause the object to be in
an stale state.
|
|
|
|
|
| |
This also mark the association as loaded given we changed it in memory
and avoid the next access to the reader to make a query to the databse.
|
|
|
|
|
|
|
|
|
|
| |
left_joins
This regression was caused by #30995 due to `Hash#fetch` won't invoke
default proc. Just revert the change since #30995 is completely fixed by
e9c1653.
Fixes #33048.
|
|
|
|
|
|
|
|
|
|
|
| |
In `_create_record`, explicit `transaction` block requires rollback
handling manually when `insert_record` is failed.
We need to handle it in `_create_record`, not in `insert_record`, since
our test cases expect a record added to target and returned even if
`insert_record` is failed,
Closes #31488.
|
|
|
|
| |
Should be done before `before_add` callbacks.
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since #31405, using `#increment!` with touch option instead of `#touch`
to touch belongs_to association if counter cache is enabled. It caused
the regression since `#increment!` won't invoke after_touch callbacks
even if touch option is given.
To fix the regression, make `#increment!` invokes after_touch callbacks
if touch option is given.
Fixes #31559.
Fixes #32408.
|