| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| ... | |
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
We added a comparison to "id", and call to `self.class.primary_key` a
*lot*. We also have performance hits from `&block` all over the place.
We skip the check in a new method, in order to avoid breaking the
behavior of `read_attribute`
|
| | | | |
| | | |
| | | |
| | | | |
Oh hey, we got to remove some code because of that!
|
| |\ \ \ \
| |/ / /
|/| | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Empact/association-bind-values-not-updated-on-save
Fix that a collection proxy could be cached before the save of the owner, resulting in an invalid proxy lacking the owner’s id
Conflicts:
activerecord/CHANGELOG.md
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
resulting in an invalid proxy lacking the owner’s id.
Absent this fix calls like: owner.association.update_all to behave unexpectedly because they try to act on association objects where
owner_id is null.
more evidence here: https://gist.github.com/Empact/5865555
```
Active Record 3.2.13
-- create_table(:firms, {:force=>true})
-> 0.1371s
-- create_table(:clients, {:force=>true})
-> 0.0005s
1 clients. 1 expected.
1 clients updated. 1 expected.
```
```
Active Record 4.0.0
-- create_table(:firms, {:force=>true})
-> 0.1606s
-- create_table(:clients, {:force=>true})
-> 0.0004s
1 clients. 1 expected.
0 clients updated. 1 expected.
```
|
| | | | | |
|
| | | | |
| | | |
| | | |
| | | | |
fixes #17495
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
if you specify a default scope on a model, it will break caching. We
cannot predict what will happen inside the scope, so play it safe for
now. fixes #17495
|
| |\ \ \ \
| | | | |
| | | | |
| | | | | |
copy reflection_scopes’s unscoped value when building scope for preloading
|
| | | | | |
| | | | |
| | | | |
| | | | | |
preloading, fixes #11036
|
| | | | | | |
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
In practical terms, this allows serialized columns and tz aware columns
to be used in wheres that go through joins, where they previously would
not behave correctly. Internally, this removes 1/3 of the cases where we
rely on Arel to perform type casting for us.
There were two non-obvious changes required for this. `update_all` on
relation was merging its bind values with arel's in the wrong order.
Additionally, through associations were assuming there would be no bind
parameters in the preloader (presumably because the where would always
be part of a join)
[Melanie Gilman & Sean Griffin]
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
This patch uniformizes warning messages. I used the most common style
already present in the code base:
* Capitalize the first word.
* End the message with a full stop.
* "Rails 5" instead of "Rails 5.0".
* Backticks for method names and inline code.
Also, converted a few long strings into the new heredoc convention.
|
| | | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | |
| | | | | |
The current style for warning messages without newlines uses
concatenation of string literals with manual trailing spaces
where needed.
Heredocs have better readability, and with `squish` we can still
produce a single line.
This is a similar use case to the one that motivated defining
`strip_heredoc`, heredocs are super clean.
|
| |/ / / /
| | | |
| | | |
| | | | |
This will avoid naming clash with user defined methods
|
| | | | |
| | | |
| | | |
| | | | |
follow up for #17052
|
| | | | |
| | | |
| | | |
| | | |
| | | | |
For now, we don't want to take "scoping" calls in to account when
calculating cache keys for relations, so just opt-out.
|
| | | | |
| | | |
| | | |
| | | |
| | | | |
emit an event when we instantiate AR objects so we can see how many
records were instantiated and how long it took
|
| |\ \ \ \
| | | | |
| | | | | |
Change `gsub` to `tr` where possible
|
| | |/ / / |
|
| |/ / /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
For detailed testing of behavior see:
https://gist.github.com/eileencodes/5b0a2fe011dcff6203fe
This shows destroy_all always destroys records and fires callbacks.
It will never use nullify or delete_all
delete_all's behavior varies greatly based on `hm` vs `hm:t` and deletion
strategy.
|
| |\ \ \
| | | |
| | | |
| | | |
| | | | |
phiggins/remove-dynamic-send-on-built-in-callbacks
Reduce allocations when running AR callbacks.
|
| | | |/
| |/|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Inspired by @tenderlove's work in
c363fff29f060e6a2effe1e4bb2c4dd4cd805d6e, this reduces the number of
strings allocated when running callbacks for ActiveRecord instances. I
measured that using this script:
```
require 'objspace'
require 'active_record'
require 'allocation_tracer'
ActiveRecord::Base.establish_connection adapter: "sqlite3",
database: ":memory:"
ActiveRecord::Base.connection.instance_eval do
create_table(:articles) { |t| t.string :name }
end
class Article < ActiveRecord::Base; end
a = Article.create name: "foo"
a = Article.find a.id
N = 10
result = ObjectSpace::AllocationTracer.trace do
N.times { Article.find a.id }
end
result.sort.each do |k,v|
p k => v
end
puts "total: #{result.values.map(&:first).inject(:+)}"
```
When I run this against master and this branch I get this output:
```
pete@balloon:~/projects/rails/activerecord$ git checkout master
M Gemfile
Switched to branch 'master'
pete@balloon:~/projects/rails/activerecord$ bundle exec ruby benchmark_allocation_with_callback_send.rb > allocations_before
pete@balloon:~/projects/rails/activerecord$ git checkout remove-dynamic-send-on-built-in-callbacks
M Gemfile
Switched to branch 'remove-dynamic-send-on-built-in-callbacks'
pete@balloon:~/projects/rails/activerecord$ bundle exec ruby benchmark_allocation_with_callback_send.rb > allocations_after
pete@balloon:~/projects/rails/activerecord$ diff allocations_before allocations_after
39d38
<
{["/home/pete/projects/rails/activesupport/lib/active_support/callbacks.rb",
81]=>[40, 0, 0, 0, 0, 0]}
42c41
< total: 630
---
> total: 590
```
In addition to this, there are two micro-optimizations present:
* Using `block.call if block` vs `yield if block_given?` when the block was being captured already.
```
pete@balloon:~/projects$ cat benchmark_block_call_vs_yield.rb
require 'benchmark/ips'
def block_capture_with_yield &block
yield if block_given?
end
def block_capture_with_call &block
block.call if block
end
def no_block_capture
yield if block_given?
end
Benchmark.ips do |b|
b.report("block_capture_with_yield") { block_capture_with_yield }
b.report("block_capture_with_call") { block_capture_with_call }
b.report("no_block_capture") { no_block_capture }
end
pete@balloon:~/projects$ ruby benchmark_block_call_vs_yield.rb
Calculating -------------------------------------
block_capture_with_yield
124979 i/100ms
block_capture_with_call
138340 i/100ms
no_block_capture 136827 i/100ms
-------------------------------------------------
block_capture_with_yield
5703108.9 (±2.4%) i/s - 28495212 in 4.999368s
block_capture_with_call
6840730.5 (±3.6%) i/s - 34169980 in 5.002649s
no_block_capture 5821141.4 (±2.8%) i/s - 29144151 in 5.010580s
```
* Defining and calling methods instead of using send.
```
pete@balloon:~/projects$ cat benchmark_method_call_vs_send.rb
require 'benchmark/ips'
class Foo
def tacos
nil
end
end
my_foo = Foo.new
Benchmark.ips do |b|
b.report('send') { my_foo.send('tacos') }
b.report('call') { my_foo.tacos }
end
pete@balloon:~/projects$ ruby benchmark_method_call_vs_send.rb
Calculating -------------------------------------
send 97736 i/100ms
call 151142 i/100ms
-------------------------------------------------
send 2683730.3 (±2.8%) i/s - 13487568 in 5.029763s
call 8005963.9 (±2.7%) i/s - 40052630 in 5.006604s
```
The result of this is making typical ActiveRecord operations slightly faster:
https://gist.github.com/phiggins/e46e51dcc7edb45b5f98
|
| |/ /
| |
| |
| | |
@column_names_with_alias, @dynamic_methods_hash, @time_zone_column_names, and @cached_time_zone
|
| |\ \
| | |
| | | |
Updating Associations::Preloader docs
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Much of the previous documentation introduced features new in 2011. This
commit refreshes it to provide clearer code examples and spends more
time describing the normal case (preloaded associations) and less time
describing the fallback.
[ci skip]
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This is cased by 03118bc + 9b5d603. The first commit referenced the undefined
local variable `column` when it should be using `reflection.type` as the lookup
key. The second commit changed `build_arel` to not modify the `bind_values` in-
place so we need to combine the arel's `bind_values` with the relation's when
building the SQL.
Fixes #16591
Related #15821 / #15892 / 7aeca50
|
| | | |
| | |
| | |
| | |
| | |
| | | |
Eagerly loaded collection and singular associations are ignored by the StatementCache, which causes errors when the queries they generate reference columns that were not eagerly loaded.
This commit skips the creation of the StatementCache as a fix for these scenarios.
|
| |\ \ \
| | | |
| | | |
| | | |
| | | | |
eileencodes/refactor-scope_chain-on-through-refelction-to-eliminate-branch-in-eval_scope
Always add lambda to scope chain to eliminate branch in eval_scope
|
| | | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
We convert all other scopes to lambda's so it makes sense that we should
always returns a lambda on a ThroughReflection as well. This eliminates
the need to check if the scope is a Relation.
|
| |/ / /
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Remove chain from parameters, it's no longer needed since chain and i
are being passed via next_reflection
Change name of `reflection` to `owner_reflection` because of shadow
variable warning. The last reflection will always be the owner.
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Warning looked like this:
```
/Users/senny/Projects/rails/activerecord/lib/active_record/associations/association_scope.rb:142: warning: shadowing outer local variable - reflection
```
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
This breaks the two branches of the `if reflection.last` and `else`
to clearer see where the two methods can be refactored. Eventually
we hope to remove the need for these separated methods altogether.
Move the first branch outside the loop
This code doesn't need to be in the loop because it it always affects
the last chain. `get_bind_values` and `add_constraints` must match
in this context because `get_bind_values` is the caching of `add_constraints`
Use each_cons to remove need for `chain[i + 1]`
The `chain[i + 1]` is confusing because it's not immediately obvious
what it's trying to achieve. The use of `each_cons` makes it clear
we need to get the `next_reflection`.
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Using heredoc would enforce line wrapping to whatever column width we decided to
use in the code, making it difficult for the users to read on some consoles.
This does make the source code read slightly worse and a bit more error-prone,
but this seems like a fair price to pay since the primary purpose for these
messages are for the users to read and the code will not stick around for too
long.
|
| | | | |
|
| |\ \ \
| | | |
| | | | |
[ci skip] Updated documentation syntax of block parameter for rdoc
|
| | | | | |
|
| |/ / / |
|
| | | | |
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
with dynamic conditions.
Fixes #16128
This bug was introduced in https://github.com/rails/rails/commit/c35e438620f2d56562251571377995359546393d
so it's present from 4.1.2-rc1 and after.
https://github.com/rails/rails/commit/c35e438620f2d56562251571377995359546393d
merges any relation scopes passed as proc objects to the relation,
but does *not* take into account the arity of the lambda.
To reproduce: https://gist.github.com/Agis-/5f1f0d664d2cd08dfb9b
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | | |
The instance var is already saved as a string in the initialization
method of AssociationReflection.
See https://github.com/rails/rails/blob/master/activerecord/lib/active_record/reflection.rb#L273
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | | |
WARNING: don't use them! They might change or go away between future beta/RC/
patch releases!
Also added a CHANGELOG entry for this.
|
| | | | |
|
| | | | |
|
| | | |
| | |
| | |
| | |
| | |
| | | |
We shouldn't be delegating chain to ThroughAssociation since the
only place that needs to call it is `target_scope`. Instead we
can call `reflecion.chain`.
|
| |\ \ \
| |/ /
|/| |
| | |
| | | |
JackDanger/doc-fix-in-join-association-build_constraint
[doc] updating documented parameter for build_constraint
|
| | | |
| | |
| | |
| | |
| | | |
Updates documentation in line with changes made in
743b67508e2027e1d086142ccbec47a19fc943f6
|
| | | | |
|
| | | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Pushing conditionals down to through reflection
Only the through association needs the part of this conditional
that deals with belongs to and polymorphic? so that can be pushed
down into the ThroughReflection reducing the conditionals.
Remove conditional because we can delegate join keys to source reflection
Remove need for source_macro checking
By adding join_id_for to the other reflections we remove the need
to cehck against the source_macro and can reduce the conditioanl
from the original join_id_for(owner)
Using polymorphism instead of testing the source_macro
This case statement in join_association is almost exactly the same
as the original join_keys code. Testing taht theory by creating a
new join_dependency_keys(assoc_klass) method.
Refactor join_keys further to be more concise
Fixed format of "#:nodoc:" to "# :nodoc:" where I added them to this
file.
|
| |/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Now that we define the macro on the reflection type we no longer
need to check `macro == :what` on each type for `belongs_to?` or
`has_one?` etc. These now default to false unless it's defined
in the reflection class.
Reuse existing belongs_to? method to check macros
We don't need to do `:belongs_to == macro` anymore becasue we
have a `belongs_to?` method. I didn't find this being used
anywhere for `has_one?` or `collection?` since they were already
fixed.
|
| | | |
|
