| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
| |
Currently `Relation#merge` will almost fill `values` with empty values
(e.g. `other.order_values` is always true, it should be
`other.order_values.any?`). This means that `Relation#merge` always
changes `values` even if actually `values` is nothing changed. This
behavior will makes `Relation#empty_scope?` fragile. So `Relation#merge`
should avoid unnecessary changes.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Without this fix, `JoinDependency` doesn't use a custom table alias:
```
% ARCONN=sqlite3 be ruby -w -Itest test/cases/relations_test.rb -n test_using_a_custom_table_with_joins_affects_the_wheres
Using sqlite3
Run options: -n test_using_a_custom_table_with_joins_affects_the_wheres --seed 14531
E
Error:RelationTest#test_using_a_custom_table_with_joins_affects_the_wheres:
ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: posts.author_id: SELECT "omg_posts".* FROM "posts" "omg_posts" INNER JOIN "authors" ON "authors"."id" = "posts"."author_id" WHERE "omg_posts"."title" = ? LIMIT ?
```
|
|
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
Introduced low level methods #set_value and #get_value for setting query attributes:
relation.set_value(:where, {id: 1})
relation.get_value(:includes)
Used those internally when working with relation's attributes
at the abstract level
|
| |
|
|
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
|
|
|
|
|
|
|
|
|
| |
The code that set the from clause was removed in
bdc5141652770fd227455681cde1f9899f55b0b9. I did not give any reason for
doing so. My assumption was that I intended to change it to use the
clause objects, but forgot. We appeared to not have test coverage for
this case.
Fixes #22996
|
|
|
|
|
|
| |
Since the strings are dynamically computed from a constant, the actual
strings we're creating are a known set. We can compute them ahead of
time, and reduce the number of allocations in that method.
|
|
|
|
|
|
|
|
|
| |
When preload is used in a default scope the preload_values were
returning nested arrays and causing the preloader to fail because it
doesn't know how to deal with nested arrays. So before calling preload!
we need to splat the arguments.
This is not needed to includes because it flatten its arguments.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Couldn't find other way to get the association name from a given class
other than looping through `reflect_on_all_associations` reflections ..
Noticed this one while looking at this example:
```ruby
class Product < ActiveRecord::Base
has_many :variants
has_many :translations
end
class Translation < ActiveRecord::Base
belongs_to :product
end
class Variant < ActiveRecord::Base
belongs_to :product
end
class BugTest < Minitest::Test
def test_merge_stuff
product = Product.create! name: 'huhu'
variant = Variant.create! product_id: product.id
Translation.create! locale: 'en', product_id: product.id
product_relation = Product.all
.preload(:translations)
.joins(:translations)
.merge(Translation.where(locale: 'en'))
.where(name: 'huhu')
assert_equal variant, Variant.joins(:product).merge(product_relation).first
end
end
```
|
| |
|
|
|
|
|
|
|
| |
Contrary to my previous commit message, it wasn't overkill, and led to
much cleaner code.
[Sean Griffin & anthonynavarre]
|
|
|
|
|
|
|
|
|
|
| |
The last place that was assigning it was when `from` is called with a
relation to use as a subquery. The implementation was actually
completely broken, and would break if you called `from` more than once,
or if you called it on a relation, which also had its own join clause,
as the bind values would get completely scrambled. The simplest solution
was to just move it into its own array, since creating a `FromClause`
class for this would be overkill.
|
|
|
|
| |
This will make it easy to add `having_clause` and `join_clause` later.
|
|
|
|
|
|
| |
This object being a black box, it knows the details of how to merge
itself with another where clause. This removes all references to where
values or bind values in `Relation::Merger`
|
| |
|
|
|
|
|
|
|
| |
Construction of relations can be a hotspot, we don't want to create one
of these in the constructor. This also allows us to do more expensive
things in the predicate builder's constructor, since it's created once
per AR::Base subclass
|
|
|
|
| |
Oh hey, we got to remove some code because of that!
|
|
|
|
| |
The value methods will default to an empty array for us automatically
|
|
|
|
|
|
|
|
| |
The change to accessing keys directly was originally added to allow
`merge` to take a hash. The implementation of `HashMerger` no longer
requires us to be doing so. Accessing the values directly makes it
impossible to change internal storage details, even if shim methods are
added temporarily
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
ActiveRecord::Relation::Merger's filter_binds method does not filter out bind
variables when one of the attribute nodes has a string name, but the other has
a symbol name, even when those names are actually equal.
This can result in there being more bind variables than placeholders in the
generated SQL. This is particularly an issue for PostgreSQL, where this is
treated as an error.
This patch changes the filter_binds method to make it convert both attribute
names to strings before comparing.
|
|
|
|
|
|
|
|
| |
Fixes #14752
Select mimics the block interface of arrays, but does not mock the
block interface for select!. This change moves the api to be a
private method, _select!.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The reverse_order method was using a flag to control if the order should
be reversed or not. Instead of using this variable just build the reverse order
inside its proper method.
This implementation was leading to an unexpected behavior when using
reverse_order and then applying reorder(nil).
Example:
Before
Post.order(:name).reverse_order.reorder(nil)
# => SELECT "posts".* FROM "posts" ORDER BY "posts"."id" DESC
After
Post.order(:name).reverse_order.reorder(nil)
# => SELECT "posts".* FROM "posts"
|
| |
|
|
|
|
| |
The original code ignores the `false` value because `false.blank? # => true`.
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The previous implementation was necessary in order to support stuff
like:
class Post < ActiveRecord::Base
default_scope where(published: true)
scope :ordered, order("created_at")
end
If we didn't evaluate the default scope at the last possible moment
before sending the SQL to the database, it would become impossible to
do:
Post.unscoped.ordered
This is because the default scope would already be bound up in the
"ordered" scope, and therefore wouldn't be removed by the
"Post.unscoped" part.
In 4.0, we have deprecated all "eager" forms of scopes. So now you must
write:
class Post < ActiveRecord::Base
default_scope { where(published: true) }
scope :ordered, -> { order("created_at") }
end
This prevents the default scope getting bound up inside the "ordered"
scope, which means we can now have a simpler/better/more natural
implementation of default scoping.
A knock on effect is that some things that didn't work properly now do.
For example it was previously impossible to use #except to remove a part
of the default scope, since the default scope was evaluated after the
call to #except.
|
| |
|
| |
|
| |
|
| |
|
| |
|
| |
|
| |
|
|
|
|
| |
the time
|
| |
|
|
|
|
| |
#join_associations.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In order to fix #10421 I need to enable merge to take an option
so that relations could be merged without making the last where
condition to win.
That fix would forever reside in 4-0-stable branch and would not be
merged to master since using scope without lambda has been deprecated.
In this commit I have extracted code into a method and I think it
makes code look better. Hence the request to merge it in both
master and 4-0-stable.
If there is any concern then this code can be merged only in
4-0-stable and that would be fine too.
|
|
|
|
| |
warning: `*' interpreted as argument prefix
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes #3002. Also see #5494.
```
class Comment < ActiveRecord::Base
belongs_to :post
end
class Author < ActiveRecord::Base
has_many :posts
end
class Post < ActiveRecord::Base
belongs_to :author
has_many :comments
end
```
`Comment.joins(:post).merge(Post.joins(:author).merge(Author.where(:name => "Joe Blogs"))).all` would
fail with `ActiveRecord::ConfigurationError: Association named 'author' was not found on Comment`.
It is failing because `all` is being called on relation which looks like this after all the merging:
`{:joins=>[:post, :author], :where=>[#<Arel::Nodes::Equality: ....}`. In this relation all the context that
`Post` was joined with `Author` is lost and hence the error that `author` was not found on `Comment`.
Ths solution is to build JoinAssociation when two relations with join information are being merged. And later
while building the arel use the previously built `JoinAssociation` record in `JoinDependency#graft` to
build the right from clause.
Thanks to Jared Armstrong (https://github.com/armstrjare) for most of the work. I ported it to make it
compatible with new code base.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This caused a bug with the new associations implementation, because now
association conditions are represented as Arel nodes internally right up
to when the whole thing gets turned to SQL.
In Rails 3.2, association conditions get turned to raw SQL early on,
which prevents Relation#merge from interfering.
The current implementation was buggy when a default_scope existed on the
target model, since we would basically end up doing:
default_scope.merge(association_scope)
If default_scope contained a where(foo: 'a') and association_scope
contained a where(foo: 'b').where(foo: 'c') then the merger would see
that the same column is representated on both sides of the merge and
collapse the wheres to all but the last: where(foo: 'c')
Now, the RHS of the merge is left alone.
Fixes #8990
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Before:
Calculating -------------------------------------
ar 87 i/100ms
-------------------------------------------------
ar 823.4 (±11.8%) i/s - 4089 in 5.070234s
After:
Calculating -------------------------------------
ar 88 i/100ms
-------------------------------------------------
ar 894.1 (±3.9%) i/s - 4488 in 5.028161s
Same test as 3a6dfca7f5f5bd45cea2f6ac348178e72423e1d5
|