mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Use bind values for joined tables in where statements
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 commit is contained in:
parent
a431df84b5
commit
10f75af933
8 changed files with 43 additions and 1 deletions
|
@ -1,3 +1,8 @@
|
||||||
|
* Queries now properly type cast values that are part of a join statement,
|
||||||
|
even when using type decorators such as `serialize`.
|
||||||
|
|
||||||
|
*Melanie Gilman & Sean Griffin*
|
||||||
|
|
||||||
* MySQL enum type lookups, with values matching another type, no longer result
|
* MySQL enum type lookups, with values matching another type, no longer result
|
||||||
in an endless loop.
|
in an endless loop.
|
||||||
|
|
||||||
|
|
|
@ -81,6 +81,7 @@ module ActiveRecord
|
||||||
unless reflection_scope.where_values.empty?
|
unless reflection_scope.where_values.empty?
|
||||||
scope.includes_values = Array(reflection_scope.values[:includes] || options[:source])
|
scope.includes_values = Array(reflection_scope.values[:includes] || options[:source])
|
||||||
scope.where_values = reflection_scope.values[:where]
|
scope.where_values = reflection_scope.values[:where]
|
||||||
|
scope.bind_values = reflection_scope.bind_values
|
||||||
end
|
end
|
||||||
|
|
||||||
scope.references! reflection_scope.values[:references]
|
scope.references! reflection_scope.values[:references]
|
||||||
|
|
|
@ -336,7 +336,7 @@ module ActiveRecord
|
||||||
stmt.wheres = arel.constraints
|
stmt.wheres = arel.constraints
|
||||||
end
|
end
|
||||||
|
|
||||||
bvs = bind_values + arel.bind_values
|
bvs = arel.bind_values + bind_values
|
||||||
@klass.connection.update stmt, 'SQL', bvs
|
@klass.connection.update stmt, 'SQL', bvs
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -980,6 +980,10 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
association_binds, non_binds = non_binds.partition do |column, value|
|
||||||
|
value.is_a?(Hash) && association_for_table(column)
|
||||||
|
end
|
||||||
|
|
||||||
new_opts = {}
|
new_opts = {}
|
||||||
binds = []
|
binds = []
|
||||||
|
|
||||||
|
@ -988,11 +992,24 @@ module ActiveRecord
|
||||||
new_opts[column] = connection.substitute_at(column)
|
new_opts[column] = connection.substitute_at(column)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
association_binds.each do |(column, value)|
|
||||||
|
association_relation = association_for_table(column).klass.send(:relation)
|
||||||
|
association_new_opts, association_bind = association_relation.send(:create_binds, value)
|
||||||
|
new_opts[column] = association_new_opts
|
||||||
|
binds += association_bind
|
||||||
|
end
|
||||||
|
|
||||||
non_binds.each { |column,value| new_opts[column] = value }
|
non_binds.each { |column,value| new_opts[column] = value }
|
||||||
|
|
||||||
[new_opts, binds]
|
[new_opts, binds]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def association_for_table(table_name)
|
||||||
|
table_name = table_name.to_s
|
||||||
|
@klass._reflect_on_association(table_name) ||
|
||||||
|
@klass._reflect_on_association(table_name.singularize)
|
||||||
|
end
|
||||||
|
|
||||||
def build_from
|
def build_from
|
||||||
opts, name = from_value
|
opts, name = from_value
|
||||||
case opts
|
case opts
|
||||||
|
|
|
@ -2,6 +2,8 @@ require "cases/helper"
|
||||||
require 'models/contact'
|
require 'models/contact'
|
||||||
require 'models/topic'
|
require 'models/topic'
|
||||||
require 'models/book'
|
require 'models/book'
|
||||||
|
require 'models/author'
|
||||||
|
require 'models/post'
|
||||||
|
|
||||||
class SerializationTest < ActiveRecord::TestCase
|
class SerializationTest < ActiveRecord::TestCase
|
||||||
fixtures :books
|
fixtures :books
|
||||||
|
@ -92,4 +94,11 @@ class SerializationTest < ActiveRecord::TestCase
|
||||||
book = klazz.find(books(:awdr).id)
|
book = klazz.find(books(:awdr).id)
|
||||||
assert_equal 'paperback', book.read_attribute_for_serialization(:format)
|
assert_equal 'paperback', book.read_attribute_for_serialization(:format)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_find_records_by_serialized_attributes_through_join
|
||||||
|
author = Author.create!(name: "David")
|
||||||
|
author.serialized_posts.create!(title: "Hello")
|
||||||
|
|
||||||
|
assert_equal 1, Author.joins(:serialized_posts).where(name: "David", serialized_posts: { title: "Hello" }).length
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1,5 +1,6 @@
|
||||||
class Author < ActiveRecord::Base
|
class Author < ActiveRecord::Base
|
||||||
has_many :posts
|
has_many :posts
|
||||||
|
has_many :serialized_posts
|
||||||
has_one :post
|
has_one :post
|
||||||
has_many :very_special_comments, :through => :posts
|
has_many :very_special_comments, :through => :posts
|
||||||
has_many :posts_with_comments, -> { includes(:comments) }, :class_name => "Post"
|
has_many :posts_with_comments, -> { includes(:comments) }, :class_name => "Post"
|
||||||
|
|
|
@ -233,3 +233,7 @@ class PostWithCommentWithDefaultScopeReferencesAssociation < ActiveRecord::Base
|
||||||
has_many :comment_with_default_scope_references_associations, foreign_key: :post_id
|
has_many :comment_with_default_scope_references_associations, foreign_key: :post_id
|
||||||
has_one :first_comment, class_name: "CommentWithDefaultScopeReferencesAssociation", foreign_key: :post_id
|
has_one :first_comment, class_name: "CommentWithDefaultScopeReferencesAssociation", foreign_key: :post_id
|
||||||
end
|
end
|
||||||
|
|
||||||
|
class SerializedPost < ActiveRecord::Base
|
||||||
|
serialize :title
|
||||||
|
end
|
||||||
|
|
|
@ -585,6 +585,11 @@ ActiveRecord::Schema.define do
|
||||||
t.integer :tags_with_nullify_count, default: 0
|
t.integer :tags_with_nullify_count, default: 0
|
||||||
end
|
end
|
||||||
|
|
||||||
|
create_table :serialized_posts, force: true do |t|
|
||||||
|
t.integer :author_id
|
||||||
|
t.string :title, null: false
|
||||||
|
end
|
||||||
|
|
||||||
create_table :price_estimates, force: true do |t|
|
create_table :price_estimates, force: true do |t|
|
||||||
t.string :estimate_of_type
|
t.string :estimate_of_type
|
||||||
t.integer :estimate_of_id
|
t.integer :estimate_of_id
|
||||||
|
|
Loading…
Reference in a new issue