| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
At present, ActiveRecord::Delegation compiles delegation methods on a
global basis. The compiled methods apply to all subsequent Relation
instances. This creates several problems:
1) After Post.all.recent has been called, User.all.respond_to?(:recent)
will be true, even if User.all.recent will actually raise an error due
to no User.recent method existing. (See #8080.)
2) Depending on the AR class, the delegation should do different things.
For example, if a Post.zip method exists, then Post.all.zip should call
it. But this will then result in User.zip being called by a subsequent
User.all.zip, even if User.zip does not exist, when in fact
User.all.zip should call User.all.to_a.zip. (There are various
variants of this problem.)
We are creating these compiled delegations in order to avoid method
missing and to avoid repeating logic on each invocation.
One way of handling these issues is to add additional checks in various
places to ensure we're doing the "right thing". However, this makes the
compiled methods signficantly slower. In which case, there's almost no
point in avoiding method_missing at all. (See #8127 for a proposed
solution which takes this approach.)
This is an alternative approach which involves creating a subclass of
ActiveRecord::Relation for each AR class represented. So, with this
patch, Post.all.class != User.all.class. This means that the delegations
are compiled for and only apply to a single AR class. A compiled method
for Post.all will not be invoked from User.all.
This solves the above issues without incurring significant performance
penalties. It's designed to be relatively seamless, however the downside
is a bit of complexity and potentially confusion for a user who thinks
that Post.all and User.all should be instances of the same class.
Benchmark
---------
require 'active_record'
require 'benchmark/ips'
class Post < ActiveRecord::Base
establish_connection adapter: 'sqlite3', database: ':memory:'
connection.create_table :posts
def self.omg
:omg
end
end
relation = Post.all
Benchmark.ips do |r|
r.report('delegation') { relation.omg }
r.report('constructing') { Post.all }
end
Before
------
Calculating -------------------------------------
delegation 4392 i/100ms
constructing 4780 i/100ms
-------------------------------------------------
delegation 144235.9 (±27.7%) i/s - 663192 in 5.038075s
constructing 182015.5 (±21.2%) i/s - 850840 in 5.005364s
After
-----
Calculating -------------------------------------
delegation 6677 i/100ms
constructing 6260 i/100ms
-------------------------------------------------
delegation 166828.2 (±34.2%) i/s - 754501 in 5.001430s
constructing 116575.5 (±18.6%) i/s - 563400 in 5.036690s
Comments
--------
Bear in mind that the standard deviations in the above are huge, so we
can't compare the numbers too directly. However, we can conclude that
Relation construction has become a little slower (as we'd expect), but
not by a huge huge amount, and we can still construct a large number of
Relations quite quickly.
|
| |
|
|
|
|
|
| |
Since edd94cee9af1688dd036fc58fd405adb30a5e0da, CollectionProxy
delegates all calculation methods - except count - to the scope,
which does basically what this method was doing, but since we're
delegating from the proxy, the association method was never called.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This allows us to avoid hacks like the "return 0 if owner.new_record?"
in #count (which this commit removes).
Also, the relevant foreign key may actually be present even on a new
owner record, in which case we *don't* want a null relation. This logic
is encapsulated in the #null_scope? method.
We also need to make sure that the CollectionProxy is not 'infected'
with the NullRelation module, or else the methods from there will
override the definitions in CollectionProxy, leading to incorrect
results. Hence the nullify: false option to CollectionAssociation#scope.
(This feels a bit nasty but I can't think of a better way.)
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When calling first(n) or last(n) in a collection, Active Record was
improperly trying to set the inverse of instance in case that option
existed. This change was introduced by
fdf4eae506fa9895e831f569bed3c4aa6a999a22.
In such cases we don't need to do that "manually", since the way
collection will be loaded will already handle that, so we just skip
setting the inverse association when any argument is given to
first(n)/last(n).
The test included ensures that these scenarios will have the inverse of
instance set properly.
Fixes #8087, Closes #8094.
Squashed cherry-pick from d37d40b and c368b66.
Conflicts:
activerecord/CHANGELOG.md
activerecord/lib/active_record/associations/collection_association.rb
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
If you create a new record via a collection association proxy that has
not loaded its target, and which selects additional attributes through
the association, then when the proxy loads its target, it will
inadvertently trigger an ActiveModel::MissingAttributeError during
attribute writing when CollectionAssociation#merge_target_lists attempts
to do its thing, since the newly loaded records will possess attributes
the created record does not.
This error also raises a bogus/confusing deprecation warning when
accessing the association in Rails 3.2.x, so cherry-pick would be
appreciated!
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Patches `CollectionAssociation#count` to return 0 without querying
if the parent record is new. Consider the following code:
class Account
has_many :dossiers
end
class Dossier
belongs_to :account
end
a = Account.new
a.dossiers.build
# before patch
a.dossiers.count
# SELECT COUNT(*) FROM "dossiers" WHERE "dossiers"."account_id" IS NULL
# => 0
# after
a.dosiers.count # fires without sql query
# => 0
Fixes #1856.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The COUNT clause of a finder_sql relationship is being rewritten from
COUNT(*) to COUNT(table_name.*). This does not appear to be valid syntax
in MySQL:
```
mysql> SELECT COUNT( table_name.* ) FROM `table_name`;
ERROR 1064 (42000): You have an error in your SQL syntax; check the
manual that corresponds to your MySQL server version for the right
syntax to use near '* ) FROM `table_name`' at line 1
```
This fixes the bug, as well as adding tests so we don't re-introduce
it in the future.
Fixes #3956.
|
| | |
|
| |
|
|
|
| |
COUNT(*) queries can be slow in PostgreSQL, #exists? avoids this by
selecting a single record.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
| |
This reverts commit 3803fcce26b837c0117f7d278b83c366dc4ed370.
Conflicts:
activerecord/CHANGELOG.md
It will be deprecated only in 4.0, and removed properly in 4.1.
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
| |
This code is broken (it should say association_scope.uniq_value rather
than options[:uniq]) but the tests still pass.
I think it is designed to uniq-ify associations using finder_sql.
However, I am about to remove that anyway.
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I found the next issue between CollectionAssociation `delete`
and `destroy`.
class Person < ActiveRecord::Base
has_many :pets
end
person.pets.destroy(1)
# => OK, returns the destroyed object
person.pets.destroy("2")
# => OK, returns the destroyed object
person.pets.delete(1)
# => ActiveRecord::AssociationTypeMismatch
person.pets.delete("2")
# => ActiveRecord::AssociationTypeMismatch
Adding support for deleting with a fixnum or string like
`destroy` method.
|
| |\ |
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| | |
The fix commited in e2a070c was returning the `new_target`, as
a try to return whatever user replaced association with. The
problem is, the resulting association target may be ordered
differently. In such case we want to return the target that
will be later used for that association.
|
| | | |
|
| |/
|
|
| |
Bug #6289
|
| |\
| |
| |
| |
| | |
JonRowe/patch_uniq_has_and_belongs_to_many_when_already_loaded
When Active Record has already loaded a unique association `.size` returns the wrong number.
|
| | |
| |
| |
| | |
of the uniq array
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
This helps bring the interfaces of CollectionProxy and Relation closer
together, and reduces the delegation backflips we need to perform.
For example, first_or_create is defined thus:
class ActiveRecord::Relation
def first_or_create(...)
first || create(...)
end
end
If CollectionProxy < Relation, then post.comments.first_or_create will
hit the association's #create method which will actually add the new record
to the association, just as post.comments.create would.
With the previous delegation, post.comments.first_or_create expands to
post.comments.scoped.first_or_create, where post.comments.scoped has no
knowledge of the association.
|
| |/
|
|
|
|
|
| |
* Remove unused association_class method.
* Remove a unnecessary assignment.
* Move @updated to BelongsToAssociation that only reference this instance variable.
* Reset @stale_state at the reset method. I think this place is right place.
|
| |
|
|
| |
correct
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
| |
when fetching ids for a collection, bypass preloading
to avoid the unnecessary performance overhead
|
| | |
|
| |
|
|
| |
present but the record is unsaved
|
| |
|
|
| |
Preserve SELECT columns on the COUNT for finder_sql when possible
|
