| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
| |
Fixes #18632
|
|
|
|
|
|
|
|
|
|
|
| |
With the old implementation, the bind values were created, and then we
search the attributes for `Relation` objects, and merge them. This
completely ignores the order that the actual `where` clause will use. If
all non-relation where parameters are before the relations, it will
work. However, if we query on both a relation and a value, with the
value coming second, it breaks. The order of the hash should not affect
the final query (especially since hashes being ordered is an
implementation detail)
|
|
|
|
|
|
| |
I'm looking to introduce a `WhereClause` class to handle most of this
logic, and this method will eventually move over to there. However, this
intermediate refactoring should make that easier to do.
|
|
|
|
|
|
| |
Looking through the blame, this logic used to be when we actually
created the bind tuple. My guess is that `nil` couldn't be handled there
at that time. It can, now.
|
|
|
|
|
|
|
| |
In order to better facilitate refactoring, most places that mutated
`bind_values` have already been removed. One last spot snuck through.
Since we're no longer mutating the array, it also does not need to be
duped in `initialize_copy`.
|
|
|
|
|
|
|
|
|
|
|
| |
This is cropping up all over the place. After a brief dive, I'm really
not sure why we have `arel.bind_values` at all. A cursory grep didn't
reveal where they're actually being assigned (it's definitely in AR, not
in Arel). I'd like to dig further into it, as I'm fairly certain we
don't actually need it, we just need a way for the predicate builder to
communicate merged binds upstream.
Fixes #18414
|
| |
|
|
|
|
| |
This adresses https://github.com/rails/rails/commit/1b7aa62b184c4410c99208f71b59bbac5c5f03be#commitcomment-9147803
|
|
|
|
|
| |
Tests should still pass after removing `require 'active_support/deprecation'`
from these files since the related deprecations have been removed.
|
|
|
|
| |
These requires were added only to change deprecation message
|
| |
|
|
|
|
|
|
|
|
|
|
| |
This is no longer required now that we are injecting a type caster
object into the Arel table, with the exception of uniqueness
validations. Since it calls `ConnectionAdapter#type_cast`, the value has
already been cast for the database. We don't want Arel to attempt to
cast it further, so we need to continue wrapping it in a quoted node.
This can potentially go away when this validator is refactored to make
better use of `where` or the predicate builder.
|
|
|
|
|
|
|
| |
This API will require much less consuming code to change to accomodate
the removal of automatic type casting from Arel. As long as the
predicates are constructed using the `arel_table` off of an AR subclass,
there will be no changes that need to happen.
|
|
|
|
|
|
|
| |
Part of the larger refactoring to remove type casting from Arel. We can
inform it that we already have the right type by wrapping the value in
an `Arel::Nodes::Quoted`. This commit can be reverted when we have
removed type casting from Arel in Rail 5.1
|
|
|
|
|
|
|
| |
Part of the larger refactoring to remove type casting from Arel. We can
inform it that we already have the right type by wrapping the value in
an `Arel::Nodes::Quoted`. This commit can be reverted when we have
removed type casting from Arel in Rail 5.1
|
|
|
|
| |
Part of the larger refactoring to remove type casting from Arel.
|
|
|
|
|
|
|
| |
Part of the larger refactoring to remove type casting from Arel. We can
inform it that we already have the right type by wrapping the value in
an `Arel::Nodes::Quoted`. This commit can be reverted when we have
removed type casting from Arel in Rail 5.1
|
|
|
|
|
|
|
|
| |
Part of a larger refactoring to remove type casting from Arel.
/cc @mrgilman
[Sean Griffin & Melanie Gilman]
|
|
|
|
|
|
|
|
|
| |
A custom object is required for this, as you cannot build a range
object out of `Arel::Nodes::Quoted` objects. Depends on the changes
introduced in
https://github.com/rails/arel/commit/cf03bd45e39def057a2f63e42a3391b7d750dece
/cc @mrgilman
|
|
|
|
|
|
|
|
|
|
|
| |
As part of the larger refactoring to remove type casting from Arel, we
need to do the casting of values eagerly. The predicate builder is the
closest place that knows about the Active Record class, and can
therefore have the type information.
/cc @mrgilman
[Sean Griffin & Melanie Gilman]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This class cares far too much about the internals of other parts of
Active Record. This is an attempt to break out a meaningful object which
represents the needs of the predicate builder. I'm not fully satisfied
with the name, but the general concept is an object which represents a
table, the associations to/from that table, and the types associated
with it. Many of these exist at the `ActiveRecord::Base` class level,
not as properties of the table itself, hence the need for another
object. Currently it provides these by holding a reference to the class,
but that will likely change in the future. This allows the predicate
builder to remain wholy concerned with building predicates.
/cc @mrgilman
|
|
|
|
|
|
|
|
|
|
| |
I'm attempting to remove `klass` as a dependency of the predicate
builder, in favor of an object that better represents what we're using
it for. The only part of this which doesn't fit nicely into that picture
is the check for an association being polymorphic. Since I'm not yet
sure what that is going to look like, I've moved this logic into another
class in an attempt to separate things that will change from things that
won't.
|
|
|
|
|
|
|
|
|
|
| |
This reduces the number of places which will need to care about single
value or range specific logic as we introduce type casting. The array
handler is only responsible for producing `in` statements.
/cc @mrgilman
[Sean Griffin & Melanie Gilman]
|
|
|
|
|
|
|
|
|
|
|
|
| |
This will allow us to pass the predicate builder into the constructor of
these handlers. The procs had to be changed to objects, because the
`PredicateBuilder` needs to be marshalable. If we ever decide to make
`register_handler` part of the public API, we should come up with a
better solution which allows procs.
/cc @mrgilman
[Sean Griffin & Melanie Gilman]
|
|
|
|
|
| |
We're accidentally documenting `PredicateBuilder` and `ArrayHandler`
since there's a constant which is missing `# :nodoc:`
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
| |
I find that `Specifies the starting point for the batch processing.`
does not give enough information for me to understand what this
parameter actually does.
|
| |
|
| |
|
|
|
|
| |
Users should pass strings to queries instead of classes
|
|
|
|
| |
key, not just integer ones, as per @a58cafeb3a86be46849de57481b6644094fb8165
|
|
|
|
|
|
|
|
|
|
| |
This ensures that we're handling all forms of nested tables the same way.
We're aware that the `convert_dot_notation_to_hash` method will cause a
performance hit, and we intend to come back to it once we've refactored some of
the surrounding code.
[Melissa Xie & Melanie Gilman]
|
|\
| |
| | |
Refactor `PredicateBuilder` from singleton to instance
|
| | |
|
|/
|
|
|
|
| |
This commit fixes the following case.
User.where(User.arel_table[:created_at].lteq(1.year.ago)).unscope(where :created_at)
|
|
|
|
|
| |
`where_sql` now requires that we pass it an engine. None of the manager
classes take an engine in their constructor.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We never actually make use of it on the table, since we're constructing
the select manager manually. It looks like if we ever actually were
grabbing it from the table, we're grossly misusing it since it's meant
to vary by AR class.
Its existence on `Arel::Table` appears to be purely for convenience
methods that are never used outside of tests. However, in production
code it just complicates construction of the tables on the rails side,
and the plan is to remove it from `Arel::Table` entirely. I'm not
convinced it needs to live on `SelectManager`, etc either.
|
| |
|
|
|
|
| |
on the joined assoiciation
|
| |
|
|
|
|
| |
Oh hey, we got to remove some code because of that!
|
|
|
|
|
|
| |
Arel handles this for us automatically. Updated tests, as BindParam is
no longer a subclass of SqlLiteral. We should remove the second argument
to substitute_at entirely, as it's no longer used
|
|
|
|
|
| |
We should not behave differently just because a class has a default
scope.
|
|
|
|
|
|
| |
Arel specifically handles `SelectManager`, with the same logic we're
currently performing. The AST is `Enumerable`, which Arel looks for
separately now.
|
| |
|
|
|
|
|
|
| |
We need to re-order the bind parameters since the AST returned by the
relation will have the where statement as the first bp, which breaks on
PG.
|
|
|
|
|
| |
MySQL reports the column name as `"MAX(developer_id)"`. PG will report
it as `"max"`
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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]
|
|
|
|
|
|
| |
We end up re-ordering them either way when we construct the Arel AST (in order
to deal with rewhere, etc), so we shouldn't bother giving it a number in the
first place beforehand.
|