diff options
27 files changed, 295 insertions, 244 deletions
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 236d36e15f..75795fe493 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -180,14 +180,15 @@ module ActiveRecord end if has_include?(column_names.first) - construct_relation_for_association_calculations.pluck(*column_names) + relation = apply_join_dependency + relation.pluck(*column_names) else - enforce_raw_sql_whitelist(column_names) + enforce_raw_sql_whitelist(column_names, whitelist: allowed_pluck_columns) relation = spawn relation.select_values = column_names.map { |cn| @klass.respond_to_attribute?(cn) ? arel_attribute(cn) : cn } - result = klass.connection.select_all(relation.arel, nil, bound_attributes) + result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) } result.cast_values(klass.attribute_types) end end @@ -202,26 +203,10 @@ module ActiveRecord private - def _pluck(column_names, unsafe_raw) - unrecognized = column_names.reject do |cn| - @klass.respond_to_attribute?(cn) - end - - if loaded? && unrecognized.none? - records.pluck(*column_names) - elsif has_include?(column_names.first) - relation = apply_join_dependency - relation.pluck(*column_names) - elsif unsafe_raw || unrecognized.none? - relation = spawn - relation.select_values = column_names.map { |cn| - @klass.respond_to_attribute?(cn) ? arel_attribute(cn) : cn - } - result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) } - result.cast_values(klass.attribute_types) - else - raise ArgumentError, "Invalid column name: #{unrecognized}" - end + def allowed_pluck_columns + @klass.attribute_names_and_aliases.map do |name| + [name, "#{table_name}.#{name}"] + end.flatten end def has_include?(column_name) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index f3b44d19d6..094e5aa733 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -297,7 +297,11 @@ module ActiveRecord # Same as #order but operates on relation in-place instead of copying. def order!(*args) # :nodoc: - @klass.enforce_raw_sql_whitelist(column_names_from_order_arguments(args)) + @klass.enforce_raw_sql_whitelist( + column_names_from_order_arguments(args), + whitelist: allowed_order_columns + ) + preprocess_order_args(args) self.order_values += args @@ -320,7 +324,11 @@ module ActiveRecord # Same as #reorder but operates on relation in-place instead of copying. def reorder!(*args) # :nodoc: - @klass.enforce_raw_sql_whitelist(column_names_from_order_arguments(args)) + @klass.enforce_raw_sql_whitelist( + column_names_from_order_arguments(args), + whitelist: allowed_order_columns + ) + preprocess_order_args(args) self.reordering_value = true @@ -920,6 +928,20 @@ module ActiveRecord private + def allowed_order_columns + @klass.attribute_names_and_aliases.map do |name| + [name, "#{table_name}.#{name}"].map do |name| + [ + name, + "#{name} asc", + "#{name} ASC", + "#{name} desc", + "#{name} DESC" + ] + end + end.flatten + end + # Extract column names from arguments passed to #order or #reorder. def column_names_from_order_arguments(args) args.flat_map { |arg| arg.is_a?(Hash) ? arg.keys : arg } diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 3eba5ed466..f08fd73da4 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -18,7 +18,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase :categorizations, :people, :categories, :edges, :vertices def test_eager_association_loading_with_cascaded_two_levels - authors = Author.all.merge!(includes: { posts: :comments }, order: Arel.sql("authors.id")).to_a + authors = Author.all.merge!(includes: { posts: :comments }, order: "authors.id").to_a assert_equal 3, authors.size assert_equal 5, authors[0].posts.size assert_equal 3, authors[1].posts.size @@ -26,7 +26,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_cascaded_two_levels_and_one_level - authors = Author.all.merge!(includes: [{ posts: :comments }, :categorizations], order: Arel.sql("authors.id")).to_a + authors = Author.all.merge!(includes: [{ posts: :comments }, :categorizations], order: "authors.id").to_a assert_equal 3, authors.size assert_equal 5, authors[0].posts.size assert_equal 3, authors[1].posts.size @@ -42,7 +42,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_grafts_stashed_associations_to_correct_parent - assert_equal people(:michael), Person.eager_load(primary_contact: :primary_contact).where("primary_contacts_people_2.first_name = ?", "Susan").order(Arel.sql("people.id")).first + assert_equal people(:michael), Person.eager_load(primary_contact: :primary_contact).where("primary_contacts_people_2.first_name = ?", "Susan").order("people.id").first end def test_cascaded_eager_association_loading_with_join_for_count @@ -78,7 +78,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_cascaded_two_levels_with_two_has_many_associations - authors = Author.all.merge!(includes: { posts: [:comments, :categorizations] }, order: Arel.sql("authors.id")).to_a + authors = Author.all.merge!(includes: { posts: [:comments, :categorizations] }, order: "authors.id").to_a assert_equal 3, authors.size assert_equal 5, authors[0].posts.size assert_equal 3, authors[1].posts.size @@ -86,7 +86,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_cascaded_two_levels_and_self_table_reference - authors = Author.all.merge!(includes: { posts: [:comments, :author] }, order: Arel.sql("authors.id")).to_a + authors = Author.all.merge!(includes: { posts: [:comments, :author] }, order: "authors.id").to_a assert_equal 3, authors.size assert_equal 5, authors[0].posts.size assert_equal authors(:david).name, authors[0].name @@ -94,13 +94,13 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_cascaded_two_levels_with_condition - authors = Author.all.merge!(includes: { posts: :comments }, where: "authors.id=1", order: Arel.sql("authors.id")).to_a + authors = Author.all.merge!(includes: { posts: :comments }, where: "authors.id=1", order: "authors.id").to_a assert_equal 1, authors.size assert_equal 5, authors[0].posts.size end def test_eager_association_loading_with_cascaded_three_levels_by_ping_pong - firms = Firm.all.merge!(includes: { account: { firm: :account } }, order: Arel.sql("companies.id")).to_a + firms = Firm.all.merge!(includes: { account: { firm: :account } }, order: "companies.id").to_a assert_equal 2, firms.size assert_equal firms.first.account, firms.first.account.firm.account assert_equal companies(:first_firm).account, assert_no_queries { firms.first.account.firm.account } @@ -108,7 +108,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_has_many_sti - topics = Topic.all.merge!(includes: :replies, order: Arel.sql("topics.id")).to_a + topics = Topic.all.merge!(includes: :replies, order: "topics.id").to_a first, second, = topics(:first).replies.size, topics(:second).replies.size assert_no_queries do assert_equal first, topics[0].replies.size @@ -121,7 +121,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase silly.parent_id = 1 assert silly.save - topics = Topic.all.merge!(includes: :replies, order: [Arel.sql("topics.id"), Arel.sql("replies_topics.id")]).to_a + topics = Topic.all.merge!(includes: :replies, order: ["topics.id", Arel.sql("replies_topics.id")]).to_a assert_no_queries do assert_equal 2, topics[0].replies.size assert_equal 0, topics[1].replies.size @@ -136,7 +136,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_multiple_stis_and_order - author = Author.all.merge!(includes: { posts: [ :special_comments , :very_special_comment ] }, order: [Arel.sql("authors.name"), Arel.sql("comments.body"), Arel.sql("very_special_comments_posts.body")], where: "posts.id = 4").first + author = Author.all.merge!(includes: { posts: [ :special_comments , :very_special_comment ] }, order: ["authors.name", Arel.sql("comments.body"), Arel.sql("very_special_comments_posts.body")], where: "posts.id = 4").first assert_equal authors(:david), author assert_no_queries do author.posts.first.special_comments @@ -154,7 +154,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_where_first_level_returns_nil - authors = Author.all.merge!(includes: { post_about_thinking: :comments }, order: Arel.sql("authors.id DESC")).to_a + authors = Author.all.merge!(includes: { post_about_thinking: :comments }, order: "authors.id DESC").to_a assert_equal [authors(:bob), authors(:mary), authors(:david)], authors assert_no_queries do authors[2].post_about_thinking.comments.first @@ -162,17 +162,17 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase end def test_eager_association_loading_with_recursive_cascading_four_levels_has_many_through - source = Vertex.all.merge!(includes: { sinks: { sinks: { sinks: :sinks } } }, order: Arel.sql("vertices.id")).first + source = Vertex.all.merge!(includes: { sinks: { sinks: { sinks: :sinks } } }, order: "vertices.id").first assert_equal vertices(:vertex_4), assert_no_queries { source.sinks.first.sinks.first.sinks.first } end def test_eager_association_loading_with_recursive_cascading_four_levels_has_and_belongs_to_many - sink = Vertex.all.merge!(includes: { sources: { sources: { sources: :sources } } }, order: Arel.sql("vertices.id DESC")).first + sink = Vertex.all.merge!(includes: { sources: { sources: { sources: :sources } } }, order: "vertices.id DESC").first assert_equal vertices(:vertex_1), assert_no_queries { sink.sources.first.sources.first.sources.first.sources.first } end def test_eager_association_loading_with_cascaded_interdependent_one_level_and_two_levels - authors_relation = Author.all.merge!(includes: [:comments, { posts: :categorizations }], order: Arel.sql("authors.id")) + authors_relation = Author.all.merge!(includes: [:comments, { posts: :categorizations }], order: "authors.id") authors = authors_relation.to_a assert_equal 3, authors.size assert_equal 10, authors[0].comments.size diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index d5ca87900e..75f851fec7 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -82,7 +82,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_with_ordering - list = Post.all.merge!(includes: :comments, order: Arel.sql("posts.id DESC")).to_a + list = Post.all.merge!(includes: :comments, order: "posts.id DESC").to_a [:other_by_mary, :other_by_bob, :misc_by_mary, :misc_by_bob, :eager_other, :sti_habtm, :sti_post_and_comments, :sti_comments, :authorless, :thinking, :welcome ].each_with_index do |post, index| @@ -108,12 +108,12 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_with_two_tables_in_from_without_getting_double_quoted - posts = Post.select("posts.*").from("authors, posts").eager_load(:comments).where("posts.author_id = authors.id").order(Arel.sql("posts.id")).to_a + posts = Post.select("posts.*").from("authors, posts").eager_load(:comments).where("posts.author_id = authors.id").order("posts.id").to_a assert_equal 2, posts.first.comments.size end def test_loading_with_multiple_associations - posts = Post.all.merge!(includes: [ :comments, :author, :categories ], order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(includes: [ :comments, :author, :categories ], order: "posts.id").to_a assert_equal 2, posts.first.comments.size assert_equal 2, posts.first.categories.size assert_includes posts.first.comments, comments(:greetings) @@ -279,7 +279,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_loading_from_an_association - posts = authors(:david).posts.merge(includes: :comments, order: Arel.sql("posts.id")).to_a + posts = authors(:david).posts.merge(includes: :comments, order: "posts.id").to_a assert_equal 2, posts.first.comments.size end @@ -312,7 +312,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_nested_loading_through_has_one_association_with_order - aa = AuthorAddress.all.merge!(includes: { author: :posts }, order: Arel.sql("author_addresses.id")).find(author_addresses(:david_address).id) + aa = AuthorAddress.all.merge!(includes: { author: :posts }, order: "author_addresses.id").find(author_addresses(:david_address).id) assert_equal aa.author.posts.count, aa.author.posts.length end @@ -364,31 +364,31 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_association_loading_with_belongs_to_and_limit - comments = Comment.all.merge!(includes: :post, limit: 5, order: Arel.sql("comments.id")).to_a + comments = Comment.all.merge!(includes: :post, limit: 5, order: "comments.id").to_a assert_equal 5, comments.length assert_equal [1, 2, 3, 5, 6], comments.collect(&:id) end def test_eager_association_loading_with_belongs_to_and_limit_and_conditions - comments = Comment.all.merge!(includes: :post, where: "post_id = 4", limit: 3, order: Arel.sql("comments.id")).to_a + comments = Comment.all.merge!(includes: :post, where: "post_id = 4", limit: 3, order: "comments.id").to_a assert_equal 3, comments.length assert_equal [5, 6, 7], comments.collect(&:id) end def test_eager_association_loading_with_belongs_to_and_limit_and_offset - comments = Comment.all.merge!(includes: :post, limit: 3, offset: 2, order: Arel.sql("comments.id")).to_a + comments = Comment.all.merge!(includes: :post, limit: 3, offset: 2, order: "comments.id").to_a assert_equal 3, comments.length assert_equal [3, 5, 6], comments.collect(&:id) end def test_eager_association_loading_with_belongs_to_and_limit_and_offset_and_conditions - comments = Comment.all.merge!(includes: :post, where: "post_id = 4", limit: 3, offset: 1, order: Arel.sql("comments.id")).to_a + comments = Comment.all.merge!(includes: :post, where: "post_id = 4", limit: 3, offset: 1, order: "comments.id").to_a assert_equal 3, comments.length assert_equal [6, 7, 8], comments.collect(&:id) end def test_eager_association_loading_with_belongs_to_and_limit_and_offset_and_conditions_array - comments = Comment.all.merge!(includes: :post, where: ["post_id = ?", 4], limit: 3, offset: 1, order: Arel.sql("comments.id")).to_a + comments = Comment.all.merge!(includes: :post, where: ["post_id = ?", 4], limit: 3, offset: 1, order: "comments.id").to_a assert_equal 3, comments.length assert_equal [6, 7, 8], comments.collect(&:id) end @@ -402,7 +402,7 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_eager_association_loading_with_belongs_to_and_conditions_hash comments = [] assert_nothing_raised do - comments = Comment.all.merge!(includes: :post, where: { posts: { id: 4 } }, limit: 3, order: Arel.sql("comments.id")).to_a + comments = Comment.all.merge!(includes: :post, where: { posts: { id: 4 } }, limit: 3, order: "comments.id").to_a end assert_equal 3, comments.length assert_equal [5, 6, 7], comments.collect(&:id) @@ -432,13 +432,13 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_association_loading_with_belongs_to_and_limit_and_multiple_associations - posts = Post.all.merge!(includes: [:author, :very_special_comment], limit: 1, order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(includes: [:author, :very_special_comment], limit: 1, order: "posts.id").to_a assert_equal 1, posts.length assert_equal [1], posts.collect(&:id) end def test_eager_association_loading_with_belongs_to_and_limit_and_offset_and_multiple_associations - posts = Post.all.merge!(includes: [:author, :very_special_comment], limit: 1, offset: 1, order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(includes: [:author, :very_special_comment], limit: 1, offset: 1, order: "posts.id").to_a assert_equal 1, posts.length assert_equal [2], posts.collect(&:id) end @@ -508,9 +508,9 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_with_has_many_through - posts_with_comments = people(:michael).posts.merge(includes: :comments, order: Arel.sql("posts.id")).to_a - posts_with_author = people(:michael).posts.merge(includes: :author, order: Arel.sql("posts.id")).to_a - posts_with_comments_and_author = people(:michael).posts.merge(includes: [ :comments, :author ], order: Arel.sql("posts.id")).to_a + posts_with_comments = people(:michael).posts.merge(includes: :comments, order: "posts.id").to_a + posts_with_author = people(:michael).posts.merge(includes: :author, order: "posts.id").to_a + posts_with_comments_and_author = people(:michael).posts.merge(includes: [ :comments, :author ], order: "posts.id").to_a assert_equal 2, posts_with_comments.inject(0) { |sum, post| sum + post.comments.size } assert_equal authors(:david), assert_no_queries { posts_with_author.first.author } assert_equal authors(:david), assert_no_queries { posts_with_comments_and_author.first.author } @@ -526,7 +526,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_with_has_many_through_an_sti_join_model - author = Author.all.merge!(includes: :special_post_comments, order: Arel.sql("authors.id")).first + author = Author.all.merge!(includes: :special_post_comments, order: "authors.id").first assert_equal [comments(:does_it_hurt)], assert_no_queries { author.special_post_comments } end @@ -539,14 +539,14 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_with_has_many_through_an_sti_join_model_with_conditions_on_both - author = Author.all.merge!(includes: :special_nonexistent_post_comments, order: Arel.sql("authors.id")).first + author = Author.all.merge!(includes: :special_nonexistent_post_comments, order: "authors.id").first assert_equal [], author.special_nonexistent_post_comments end def test_eager_with_has_many_through_join_model_with_conditions assert_equal Author.all.merge!(includes: :hello_post_comments, - order: Arel.sql("authors.id")).first.hello_post_comments.sort_by(&:id), - Author.all.merge!(order: Arel.sql("authors.id")).first.hello_post_comments.sort_by(&:id) + order: "authors.id").first.hello_post_comments.sort_by(&:id), + Author.all.merge!(order: "authors.id").first.hello_post_comments.sort_by(&:id) end def test_eager_with_has_many_through_join_model_with_conditions_on_top_level @@ -573,19 +573,19 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_with_has_many_and_limit - posts = Post.all.merge!(order: Arel.sql("posts.id asc"), includes: [ :author, :comments ], limit: 2).to_a + posts = Post.all.merge!(order: "posts.id asc", includes: [ :author, :comments ], limit: 2).to_a assert_equal 2, posts.size assert_equal 3, posts.inject(0) { |sum, post| sum + post.comments.size } end def test_eager_with_has_many_and_limit_and_conditions - posts = Post.all.merge!(includes: [ :author, :comments ], limit: 2, where: "posts.body = 'hello'", order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(includes: [ :author, :comments ], limit: 2, where: "posts.body = 'hello'", order: "posts.id").to_a assert_equal 2, posts.size assert_equal [4, 5], posts.collect(&:id) end def test_eager_with_has_many_and_limit_and_conditions_array - posts = Post.all.merge!(includes: [ :author, :comments ], limit: 2, where: [ "posts.body = ?", "hello" ], order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(includes: [ :author, :comments ], limit: 2, where: [ "posts.body = ?", "hello" ], order: "posts.id").to_a assert_equal 2, posts.size assert_equal [4, 5], posts.collect(&:id) end @@ -643,7 +643,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_with_has_and_belongs_to_many_and_limit - posts = Post.all.merge!(includes: :categories, order: Arel.sql("posts.id"), limit: 3).to_a + posts = Post.all.merge!(includes: :categories, order: "posts.id", limit: 3).to_a assert_equal 3, posts.size assert_equal 2, posts[0].categories.size assert_equal 1, posts[1].categories.size @@ -709,7 +709,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_association_loading_with_habtm - posts = Post.all.merge!(includes: :categories, order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(includes: :categories, order: "posts.id").to_a assert_equal 2, posts[0].categories.size assert_equal 1, posts[1].categories.size assert_equal 0, posts[2].categories.size @@ -891,14 +891,14 @@ class EagerAssociationTest < ActiveRecord::TestCase posts(:thinking, :sti_comments), Post.all.merge!( includes: [:author, :comments], where: { "authors.name" => "David" }, - order: [Arel.sql("UPPER(posts.title)"), Arel.sql("posts.id")], limit: 2, offset: 1 + order: [Arel.sql("UPPER(posts.title)"), "posts.id"], limit: 2, offset: 1 ).to_a ) assert_equal( posts(:sti_post_and_comments, :sti_comments), Post.all.merge!( includes: [:author, :comments], where: { "authors.name" => "David" }, - order: [Arel.sql("UPPER(posts.title) DESC"), Arel.sql("posts.id")], limit: 2, offset: 1 + order: [Arel.sql("UPPER(posts.title) DESC"), "posts.id"], limit: 2, offset: 1 ).to_a ) end @@ -909,7 +909,7 @@ class EagerAssociationTest < ActiveRecord::TestCase Person.references(:number1_fans_people).merge( includes: [:readers, :primary_contact, :number1_fan], where: "number1_fans_people.first_name like 'M%'", - order: Arel.sql("people.id"), limit: 2, offset: 0 + order: "people.id", limit: 2, offset: 0 ).to_a ) end @@ -1102,18 +1102,18 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_eager_loading_with_conditions_on_joined_table_preloads posts = assert_queries(2) do - Post.all.merge!(select: "distinct posts.*", includes: :author, joins: [:comments], where: "comments.body like 'Thank you%'", order: Arel.sql("posts.id")).to_a + Post.all.merge!(select: "distinct posts.*", includes: :author, joins: [:comments], where: "comments.body like 'Thank you%'", order: "posts.id").to_a end assert_equal [posts(:welcome)], posts assert_equal authors(:david), assert_no_queries { posts[0].author } posts = assert_queries(2) do - Post.all.merge!(includes: :author, joins: { taggings: :tag }, where: "tags.name = 'General'", order: Arel.sql("posts.id")).to_a + Post.all.merge!(includes: :author, joins: { taggings: :tag }, where: "tags.name = 'General'", order: "posts.id").to_a end assert_equal posts(:welcome, :thinking), posts posts = assert_queries(2) do - Post.all.merge!(includes: :author, joins: { taggings: { tag: :taggings } }, where: "taggings_tags.super_tag_id=2", order: Arel.sql("posts.id")).to_a + Post.all.merge!(includes: :author, joins: { taggings: { tag: :taggings } }, where: "taggings_tags.super_tag_id=2", order: "posts.id").to_a end assert_equal posts(:welcome, :thinking), posts end @@ -1132,13 +1132,13 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_eager_loading_with_conditions_on_string_joined_table_preloads posts = assert_queries(2) do - Post.all.merge!(select: "distinct posts.*", includes: :author, joins: "INNER JOIN comments on comments.post_id = posts.id", where: "comments.body like 'Thank you%'", order: Arel.sql("posts.id")).to_a + Post.all.merge!(select: "distinct posts.*", includes: :author, joins: "INNER JOIN comments on comments.post_id = posts.id", where: "comments.body like 'Thank you%'", order: "posts.id").to_a end assert_equal [posts(:welcome)], posts assert_equal authors(:david), assert_no_queries { posts[0].author } posts = assert_queries(2) do - Post.all.merge!(select: "distinct posts.*", includes: :author, joins: ["INNER JOIN comments on comments.post_id = posts.id"], where: "comments.body like 'Thank you%'", order: Arel.sql("posts.id")).to_a + Post.all.merge!(select: "distinct posts.*", includes: :author, joins: ["INNER JOIN comments on comments.post_id = posts.id"], where: "comments.body like 'Thank you%'", order: "posts.id").to_a end assert_equal [posts(:welcome)], posts assert_equal authors(:david), assert_no_queries { posts[0].author } @@ -1146,7 +1146,7 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_eager_loading_with_select_on_joined_table_preloads posts = assert_queries(2) do - Post.all.merge!(select: "posts.*, authors.name as author_name", includes: :comments, joins: :author, order: Arel.sql("posts.id")).to_a + Post.all.merge!(select: "posts.*, authors.name as author_name", includes: :comments, joins: :author, order: "posts.id").to_a end assert_equal "David", posts[0].author_name assert_equal posts(:welcome).comments, assert_no_queries { posts[0].comments } @@ -1199,7 +1199,7 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_preload_has_one_using_primary_key expected = accounts(:signals37) - firm = Firm.all.merge!(includes: :account_using_primary_key, order: Arel.sql("companies.id")).first + firm = Firm.all.merge!(includes: :account_using_primary_key, order: "companies.id").first assert_no_queries do assert_equal expected, firm.account_using_primary_key end @@ -1269,7 +1269,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_joins_with_includes_should_preload_via_joins - post = assert_queries(1) { Post.includes(:comments).joins(:comments).order(Arel.sql("posts.id desc")).to_a.first } + post = assert_queries(1) { Post.includes(:comments).joins(:comments).order("posts.id desc").to_a.first } assert_queries(0) do assert_not_equal 0, post.comments.to_a.count @@ -1284,10 +1284,10 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_deep_including_through_habtm # warm up habtm cache - posts = Post.all.merge!(includes: { categories: :categorizations }, order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(includes: { categories: :categorizations }, order: "posts.id").to_a posts[0].categories[0].categorizations.length - posts = Post.all.merge!(includes: { categories: :categorizations }, order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(includes: { categories: :categorizations }, order: "posts.id").to_a assert_no_queries { assert_equal 2, posts[0].categories[0].categorizations.length } assert_no_queries { assert_equal 1, posts[0].categories[1].categorizations.length } assert_no_queries { assert_equal 2, posts[1].categories[0].categorizations.length } @@ -1513,6 +1513,6 @@ class EagerAssociationTest < ActiveRecord::TestCase private def find_all_ordered(klass, include = nil) - klass.order(Arel.sql("#{klass.table_name}.#{klass.primary_key}")).includes(include).to_a + klass.order("#{klass.table_name}.#{klass.primary_key}").includes(include).to_a end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 85733f9056..3597da7ff3 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -48,7 +48,7 @@ class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCa author = authors(:david) # this can fail on adapters which require ORDER BY expressions to be included in the SELECT expression # if the reorder clauses are not correctly handled - assert author.posts_with_comments_sorted_by_comment_id.where("comments.id > 0").reorder(Arel.sql("posts.comments_count DESC"), Arel.sql("posts.tags_count DESC")).last + assert author.posts_with_comments_sorted_by_comment_id.where("comments.id > 0").reorder("posts.comments_count DESC", "posts.tags_count DESC").last end end @@ -1123,7 +1123,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_deleting_updates_counter_cache - topic = Topic.order(Arel.sql("id ASC")).first + topic = Topic.order("id ASC").first assert_equal topic.replies.to_a.size, topic.replies_count topic.replies.delete(topic.replies.first) @@ -1162,7 +1162,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_pushing_association_updates_counter_cache - topic = Topic.order(Arel.sql("id ASC")).first + topic = Topic.order("id ASC").first reply = Reply.create! assert_difference "topic.reload.replies_count", 1 do @@ -1212,7 +1212,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_calling_update_attributes_on_id_changes_the_counter_cache - topic = Topic.order(Arel.sql("id ASC")).first + topic = Topic.order("id ASC").first original_count = topic.replies.to_a.size assert_equal original_count, topic.replies_count diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index ed96eb54c1..046020e310 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -71,7 +71,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase def self.name; "Person"; end has_many :readers - has_many :posts, -> { order(Arel.sql("posts.id DESC")) }, through: :readers + has_many :posts, -> { order("posts.id DESC") }, through: :readers end posts = person_prime.includes(:posts).first.posts @@ -985,7 +985,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_joining_has_many_through_belongs_to - posts = Post.joins(:author_categorizations).order(Arel.sql("posts.id")). + posts = Post.joins(:author_categorizations).order("posts.id"). where("categorizations.id" => categorizations(:mary_thinking_sti).id) assert_equal [posts(:eager_other), posts(:misc_by_mary), posts(:other_by_mary)], posts diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 8f08684820..a3acb1a152 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -244,8 +244,8 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_include_has_many_through - posts = Post.all.merge!(order: Arel.sql("posts.id")).to_a - posts_with_authors = Post.all.merge!(includes: :authors, order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(order: "posts.id").to_a + posts_with_authors = Post.all.merge!(includes: :authors, order: "posts.id").to_a assert_equal posts.length, posts_with_authors.length posts.length.times do |i| assert_equal posts[i].authors.length, assert_no_queries { posts_with_authors[i].authors.length } @@ -269,8 +269,8 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_include_polymorphic_has_many_through - posts = Post.all.merge!(order: Arel.sql("posts.id")).to_a - posts_with_tags = Post.all.merge!(includes: :tags, order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(order: "posts.id").to_a + posts_with_tags = Post.all.merge!(includes: :tags, order: "posts.id").to_a assert_equal posts.length, posts_with_tags.length posts.length.times do |i| assert_equal posts[i].tags.length, assert_no_queries { posts_with_tags[i].tags.length } @@ -278,8 +278,8 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_include_polymorphic_has_many - posts = Post.all.merge!(order: Arel.sql("posts.id")).to_a - posts_with_taggings = Post.all.merge!(includes: :taggings, order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(order: "posts.id").to_a + posts_with_taggings = Post.all.merge!(includes: :taggings, order: "posts.id").to_a assert_equal posts.length, posts_with_taggings.length posts.length.times do |i| assert_equal posts[i].taggings.length, assert_no_queries { posts_with_taggings[i].taggings.length } @@ -326,7 +326,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_has_many_through_with_custom_primary_key_on_has_many_source - assert_equal [authors(:david), authors(:bob)], posts(:thinking).authors_using_custom_pk.order(Arel.sql("authors.id")) + assert_equal [authors(:david), authors(:bob)], posts(:thinking).authors_using_custom_pk.order("authors.id") end def test_belongs_to_polymorphic_with_counter_cache @@ -383,19 +383,19 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_has_many_through_has_many_find_all - assert_equal comments(:greetings), authors(:david).comments.order(Arel.sql("comments.id")).to_a.first + assert_equal comments(:greetings), authors(:david).comments.order("comments.id").to_a.first end def test_has_many_through_has_many_find_all_with_custom_class - assert_equal comments(:greetings), authors(:david).funky_comments.order(Arel.sql("comments.id")).to_a.first + assert_equal comments(:greetings), authors(:david).funky_comments.order("comments.id").to_a.first end def test_has_many_through_has_many_find_first - assert_equal comments(:greetings), authors(:david).comments.order(Arel.sql("comments.id")).first + assert_equal comments(:greetings), authors(:david).comments.order("comments.id").first end def test_has_many_through_has_many_find_conditions - options = { where: "comments.#{QUOTED_TYPE}='SpecialComment'", order: Arel.sql("comments.id") } + options = { where: "comments.#{QUOTED_TYPE}='SpecialComment'", order: "comments.id" } assert_equal comments(:does_it_hurt), authors(:david).comments.merge(options).first end @@ -661,8 +661,8 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_preload_polymorphic_has_many_through - posts = Post.all.merge!(order: Arel.sql("posts.id")).to_a - posts_with_tags = Post.all.merge!(includes: :tags, order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(order: "posts.id").to_a + posts_with_tags = Post.all.merge!(includes: :tags, order: "posts.id").to_a assert_equal posts.length, posts_with_tags.length posts.length.times do |i| assert_equal posts[i].tags.length, assert_no_queries { posts_with_tags[i].tags.length } @@ -688,8 +688,8 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_preload_polymorphic_has_many - posts = Post.all.merge!(order: Arel.sql("posts.id")).to_a - posts_with_taggings = Post.all.merge!(includes: :taggings, order: Arel.sql("posts.id")).to_a + posts = Post.all.merge!(order: "posts.id").to_a + posts_with_taggings = Post.all.merge!(includes: :taggings, order: "posts.id").to_a assert_equal posts.length, posts_with_taggings.length posts.length.times do |i| assert_equal posts[i].taggings.length, assert_no_queries { posts_with_taggings[i].taggings.length } diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index b8e8f48ae5..0254da9a99 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -86,7 +86,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase # Through: has_many through def test_has_many_through_has_many_through_with_has_many_source_reflection luke, david = subscribers(:first), subscribers(:second) - assert_equal [luke, david, david], authors(:david).subscribers.order(Arel.sql("subscribers.nick")) + assert_equal [luke, david, david], authors(:david).subscribers.order("subscribers.nick") end def test_has_many_through_has_many_through_with_has_many_source_reflection_preload @@ -156,7 +156,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase groucho_details, other_details = member_details(:groucho), member_details(:some_other_guy) assert_equal [groucho_details, other_details], - members(:groucho).organization_member_details.order(Arel.sql("member_details.id")) + members(:groucho).organization_member_details.order("member_details.id") end def test_has_many_through_has_one_with_has_many_through_source_reflection_preload @@ -187,7 +187,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase groucho_details, other_details = member_details(:groucho), member_details(:some_other_guy) assert_equal [groucho_details, other_details], - members(:groucho).organization_member_details_2.order(Arel.sql("member_details.id")) + members(:groucho).organization_member_details_2.order("member_details.id") end def test_has_many_through_has_one_through_with_has_many_source_reflection_preload @@ -218,7 +218,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase def test_has_many_through_has_many_with_has_and_belongs_to_many_source_reflection general, cooking = categories(:general), categories(:cooking) - assert_equal [general, cooking], authors(:bob).post_categories.order(Arel.sql("categories.id")) + assert_equal [general, cooking], authors(:bob).post_categories.order("categories.id") end def test_has_many_through_has_many_with_has_and_belongs_to_many_source_reflection_preload @@ -246,7 +246,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase def test_has_many_through_has_and_belongs_to_many_with_has_many_source_reflection greetings, more = comments(:greetings), comments(:more_greetings) - assert_equal [greetings, more], categories(:technology).post_comments.order(Arel.sql("comments.id")) + assert_equal [greetings, more], categories(:technology).post_comments.order("comments.id") end def test_has_many_through_has_and_belongs_to_many_with_has_many_source_reflection_preload @@ -264,7 +264,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase Category.joins(:post_comments).first assert_includes_and_joins_equal( - Category.where("comments.id" => comments(:more_greetings).id).order(Arel.sql("categories.id")), + Category.where("comments.id" => comments(:more_greetings).id).order("categories.id"), [categories(:general), categories(:technology)], :post_comments ) end @@ -275,7 +275,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase def test_has_many_through_has_many_with_has_many_through_habtm_source_reflection greetings, more = comments(:greetings), comments(:more_greetings) - assert_equal [greetings, more], authors(:bob).category_post_comments.order(Arel.sql("comments.id")) + assert_equal [greetings, more], authors(:bob).category_post_comments.order("comments.id") end def test_has_many_through_has_many_with_has_many_through_habtm_source_reflection_preload @@ -292,7 +292,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase Author.joins(:category_post_comments).first assert_includes_and_joins_equal( - Author.where("comments.id" => comments(:does_it_hurt).id).order(Arel.sql("authors.id")), + Author.where("comments.id" => comments(:does_it_hurt).id).order("authors.id"), [authors(:david), authors(:mary)], :category_post_comments ) end @@ -327,7 +327,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase welcome_general, thinking_general = taggings(:welcome_general), taggings(:thinking_general) assert_equal [welcome_general, thinking_general], - categorizations(:david_welcome_general).post_taggings.order(Arel.sql("taggings.id")) + categorizations(:david_welcome_general).post_taggings.order("taggings.id") end def test_has_many_through_belongs_to_with_has_many_through_source_reflection_preload @@ -411,7 +411,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase def test_distinct_has_many_through_a_has_many_through_association_on_through_reflection author = authors(:david) assert_equal [subscribers(:first), subscribers(:second)], - author.distinct_subscribers.order(Arel.sql("subscribers.nick")) + author.distinct_subscribers.order("subscribers.nick") end def test_nested_has_many_through_with_a_table_referenced_multiple_times @@ -436,7 +436,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_with_foreign_key_option_on_through_reflection - assert_equal [posts(:welcome), posts(:authorless)], people(:david).agents_posts.order(Arel.sql("posts.id")) + assert_equal [posts(:welcome), posts(:authorless)], people(:david).agents_posts.order("posts.id") assert_equal [authors(:david)], references(:david_unicyclist).agents_posts_authors references = Reference.joins(:agents_posts_authors).where("authors.id" => authors(:david).id) @@ -444,7 +444,7 @@ class NestedThroughAssociationsTest < ActiveRecord::TestCase end def test_has_many_through_with_foreign_key_option_on_source_reflection - assert_equal [people(:michael), people(:susan)], jobs(:unicyclist).agents.order(Arel.sql("people.id")) + assert_equal [people(:michael), people(:susan)], jobs(:unicyclist).agents.order("people.id") jobs = Job.joins(:agents) assert_equal [jobs(:unicyclist), jobs(:unicyclist)], jobs diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index e2e4aa22f4..a45b3a1644 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -439,7 +439,7 @@ class BasicsTest < ActiveRecord::TestCase if current_adapter?(:Mysql2Adapter) def test_update_all_with_order_and_limit - assert_equal 1, Topic.limit(1).order(Arel.sql("id DESC")).update_all(content: "bulk updated!") + assert_equal 1, Topic.limit(1).order("id DESC").update_all(content: "bulk updated!") end end @@ -1081,11 +1081,11 @@ class BasicsTest < ActiveRecord::TestCase def test_find_last last = Developer.last - assert_equal last, Developer.all.merge!(order: Arel.sql("id desc")).first + assert_equal last, Developer.all.merge!(order: "id desc").first end def test_last - assert_equal Developer.all.merge!(order: Arel.sql("id desc")).first, Developer.last + assert_equal Developer.all.merge!(order: "id desc").first, Developer.last end def test_all @@ -1095,17 +1095,17 @@ class BasicsTest < ActiveRecord::TestCase end def test_all_with_conditions - assert_equal Developer.all.merge!(order: Arel.sql("id desc")).to_a, Developer.order(Arel.sql("id desc")).to_a + assert_equal Developer.all.merge!(order: "id desc").to_a, Developer.order("id desc").to_a end def test_find_ordered_last - last = Developer.all.merge!(order: Arel.sql("developers.salary ASC")).last - assert_equal last, Developer.all.merge!(order: Arel.sql("developers.salary ASC")).to_a.last + last = Developer.all.merge!(order: "developers.salary ASC").last + assert_equal last, Developer.all.merge!(order: "developers.salary ASC").to_a.last end def test_find_reverse_ordered_last - last = Developer.all.merge!(order: Arel.sql("developers.salary DESC")).last - assert_equal last, Developer.all.merge!(order: Arel.sql("developers.salary DESC")).to_a.last + last = Developer.all.merge!(order: "developers.salary DESC").last + assert_equal last, Developer.all.merge!(order: "developers.salary DESC").to_a.last end def test_find_multiple_ordered_last @@ -1115,7 +1115,7 @@ class BasicsTest < ActiveRecord::TestCase def test_find_keeps_multiple_order_values combined = Developer.all.merge!(order: Arel.sql("developers.name, developers.salary")).to_a - assert_equal combined, Developer.all.merge!(order: [Arel.sql("developers.name"), Arel.sql("developers.salary")]).to_a + assert_equal combined, Developer.all.merge!(order: ["developers.name", "developers.salary"]).to_a end def test_find_keeps_multiple_group_values diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index ff345d5f0e..be8aeed5ac 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -9,7 +9,7 @@ class EachTest < ActiveRecord::TestCase fixtures :posts, :subscribers def setup - @posts = Post.order(Arel.sql("id asc")) + @posts = Post.order("id asc") @total = Post.count Post.count("id") # preheat arel's table cache end @@ -101,7 +101,7 @@ class EachTest < ActiveRecord::TestCase previous_logger = ActiveRecord::Base.logger ActiveRecord::Base.logger = nil assert_nothing_raised do - Post.order(Arel.sql("comments_count DESC")).find_each { |post| post } + Post.order("comments_count DESC").find_each { |post| post } end ensure ActiveRecord::Base.logger = previous_logger @@ -233,7 +233,7 @@ class EachTest < ActiveRecord::TestCase end def test_find_in_batches_should_use_any_column_as_primary_key - nick_order_subscribers = Subscriber.order(Arel.sql("nick asc")) + nick_order_subscribers = Subscriber.order("nick asc") start_nick = nick_order_subscribers.second.nick subscribers = [] @@ -329,7 +329,7 @@ class EachTest < ActiveRecord::TestCase end def test_in_batches_each_record_should_be_ordered_by_id - ids = Post.order(Arel.sql("id ASC")).pluck(:id) + ids = Post.order("id ASC").pluck(:id) assert_queries(6) do Post.in_batches(of: 2).each_record.with_index do |post, i| assert_equal ids[i], post.id @@ -384,7 +384,7 @@ class EachTest < ActiveRecord::TestCase end def test_in_batches_should_start_from_the_start_option - post = Post.order(Arel.sql("id ASC")).where("id >= ?", 2).first + post = Post.order("id ASC").where("id >= ?", 2).first assert_queries(2) do relation = Post.in_batches(of: 1, start: 2).first assert_equal post, relation.first @@ -392,7 +392,7 @@ class EachTest < ActiveRecord::TestCase end def test_in_batches_should_end_at_the_finish_option - post = Post.order(Arel.sql("id DESC")).where("id <= ?", 5).first + post = Post.order("id DESC").where("id <= ?", 5).first assert_queries(7) do relation = Post.in_batches(of: 1, finish: 5, load: true).reverse_each.first assert_equal post, relation.last @@ -451,7 +451,7 @@ class EachTest < ActiveRecord::TestCase end def test_in_batches_should_use_any_column_as_primary_key - nick_order_subscribers = Subscriber.order(Arel.sql("nick asc")) + nick_order_subscribers = Subscriber.order("nick asc") start_nick = nick_order_subscribers.second.nick subscribers = [] diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 65ebfcd989..344d1a6639 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -813,9 +813,9 @@ class FinderTest < ActiveRecord::TestCase end def test_hash_condition_find_with_array - p1, p2 = Post.limit(2).order(Arel.sql("id asc")).to_a - assert_equal [p1, p2], Post.where(id: [p1, p2]).order(Arel.sql("id asc")).to_a - assert_equal [p1, p2], Post.where(id: [p1, p2.id]).order(Arel.sql("id asc")).to_a + p1, p2 = Post.limit(2).order("id asc").to_a + assert_equal [p1, p2], Post.where(id: [p1, p2]).order("id asc").to_a + assert_equal [p1, p2], Post.where(id: [p1, p2.id]).order("id asc").to_a end def test_hash_condition_find_with_nil @@ -1013,7 +1013,7 @@ class FinderTest < ActiveRecord::TestCase end def test_find_by_one_attribute_with_several_options - assert_equal accounts(:unknown), Account.order(Arel.sql("id DESC")).where("id != ?", 3).find_by_credit_limit(50) + assert_equal accounts(:unknown), Account.order("id DESC").where("id != ?", 3).find_by_credit_limit(50) end def test_find_by_one_missing_attribute @@ -1041,7 +1041,7 @@ class FinderTest < ActiveRecord::TestCase assert_equal devs[2], Developer.offset(2).first assert_equal devs[-3], Developer.offset(2).last - assert_equal devs[-3], Developer.offset(2).order(Arel.sql("id DESC")).first + assert_equal devs[-3], Developer.offset(2).order("id DESC").first end def test_find_by_nil_attribute @@ -1094,9 +1094,9 @@ class FinderTest < ActiveRecord::TestCase end def test_find_by_records - p1, p2 = Post.limit(2).order(Arel.sql("id asc")).to_a - assert_equal [p1, p2], Post.where(["id in (?)", [p1, p2]]).order(Arel.sql("id asc")) - assert_equal [p1, p2], Post.where(["id in (?)", [p1, p2.id]]).order(Arel.sql("id asc")) + p1, p2 = Post.limit(2).order("id asc").to_a + assert_equal [p1, p2], Post.where(["id in (?)", [p1, p2]]).order("id asc") + assert_equal [p1, p2], Post.where(["id in (?)", [p1, p2.id]]).order("id asc") end def test_select_value @@ -1136,7 +1136,7 @@ class FinderTest < ActiveRecord::TestCase client_of = Company. where(client_of: [2, 1, nil], name: ["37signals", "Summit", "Microsoft"]). - order(Arel.sql("client_of DESC")). + order("client_of DESC"). map(&:client_of) assert_includes client_of, nil @@ -1146,7 +1146,7 @@ class FinderTest < ActiveRecord::TestCase def test_find_with_nil_inside_set_passed_for_attribute client_of = Company. where(client_of: [nil]). - order(Arel.sql("client_of DESC")). + order("client_of DESC"). map(&:client_of) assert_equal [], client_of.compact @@ -1155,7 +1155,7 @@ class FinderTest < ActiveRecord::TestCase def test_with_limiting_with_custom_select posts = Post.references(:authors).merge( includes: :author, select: 'posts.*, authors.id as "author_id"', - limit: 3, order: Arel.sql("posts.id") + limit: 3, order: "posts.id" ).to_a assert_equal 3, posts.size assert_equal [0, 1, 1], posts.map(&:author_id).sort diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index 3b35df526f..8a656d7720 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -13,10 +13,10 @@ class RelationMergingTest < ActiveRecord::TestCase fixtures :developers, :comments, :authors, :author_addresses, :posts def test_relation_merging - devs = Developer.where("salary >= 80000").merge(Developer.limit(2)).merge(Developer.order(Arel.sql("id ASC")).where("id < 3")) + devs = Developer.where("salary >= 80000").merge(Developer.limit(2)).merge(Developer.order("id ASC").where("id < 3")) assert_equal [developers(:david), developers(:jamis)], devs.to_a - dev_with_count = Developer.limit(1).merge(Developer.order(Arel.sql("id DESC"))).merge(Developer.select("developers.*")) + dev_with_count = Developer.limit(1).merge(Developer.order("id DESC")).merge(Developer.select("developers.*")) assert_equal [developers(:poor_jamis)], dev_with_count.to_a end @@ -57,7 +57,7 @@ class RelationMergingTest < ActiveRecord::TestCase end def test_relation_merging_with_locks - devs = Developer.lock.where("salary >= 80000").order(Arel.sql("id DESC")).merge(Developer.limit(2)) + devs = Developer.lock.where("salary >= 80000").order("id DESC").merge(Developer.limit(2)) assert devs.locked? end @@ -118,7 +118,7 @@ class MergingDifferentRelationsTest < ActiveRecord::TestCase test "merging where relations" do hello_by_bob = Post.where(body: "hello").joins(:author). - merge(Author.where(name: "Bob")).order(Arel.sql("posts.id")).pluck(Arel.sql("posts.id")) + merge(Author.where(name: "Bob")).order("posts.id").pluck("posts.id") assert_equal [posts(:misc_by_bob).id, posts(:other_by_bob).id], hello_by_bob diff --git a/activerecord/test/cases/relation/or_test.rb b/activerecord/test/cases/relation/or_test.rb index 2abc3b7fe8..7e418f9c7d 100644 --- a/activerecord/test/cases/relation/or_test.rb +++ b/activerecord/test/cases/relation/or_test.rb @@ -50,15 +50,15 @@ module ActiveRecord end def test_or_preserves_other_querying_methods - expected = Post.where("id = 1 or id = 2 or id = 3").order(Arel.sql("body asc")).to_a - partial = Post.order(Arel.sql("body asc")) + expected = Post.where("id = 1 or id = 2 or id = 3").order("body asc").to_a + partial = Post.order("body asc") assert_equal expected, partial.where("id = 1").or(partial.where(id: [2, 3])).to_a - assert_equal expected, Post.order(Arel.sql("body asc")).where("id = 1").or(Post.order(Arel.sql("body asc")).where(id: [2, 3])).to_a + assert_equal expected, Post.order("body asc").where("id = 1").or(Post.order("body asc").where(id: [2, 3])).to_a end def test_or_with_incompatible_relations error = assert_raises ArgumentError do - Post.order(Arel.sql("body asc")).where("id = 1").or(Post.order(Arel.sql("id desc")).where(id: [2, 3])).to_a + Post.order("body asc").where("id = 1").or(Post.order("id desc").where(id: [2, 3])).to_a end assert_equal "Relation passed to #or must be structurally compatible. Incompatible values: [:order]", error.message @@ -78,12 +78,12 @@ module ActiveRecord def test_or_with_unscope_order expected = Post.where("id = 1 or id = 2") - assert_equal expected, Post.order(Arel.sql("body asc")).where("id = 1").unscope(:order).or(Post.where("id = 2")).to_a + assert_equal expected, Post.order("body asc").where("id = 1").unscope(:order).or(Post.where("id = 2")).to_a end def test_or_with_incompatible_unscope error = assert_raises ArgumentError do - Post.order(Arel.sql("body asc")).where("id = 1").or(Post.order(Arel.sql("body asc")).where("id = 2").unscope(:order)).to_a + Post.order("body asc").where("id = 1").or(Post.order("body asc").where("id = 2").unscope(:order)).to_a end assert_equal "Relation passed to #or must be structurally compatible. Incompatible values: [:order]", error.message @@ -101,7 +101,7 @@ module ActiveRecord end def test_or_inside_named_scope - expected = Post.where("body LIKE '\%a\%' OR title LIKE ?", "%'%").order(Arel.sql("id DESC")).to_a + expected = Post.where("body LIKE '\%a\%' OR title LIKE ?", "%'%").order("id DESC").to_a assert_equal expected, Post.order(id: :desc).typographically_interesting end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 906f3499dd..ab2e432e78 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -102,7 +102,7 @@ class RelationTest < ActiveRecord::TestCase end def test_scoped_first - topics = Topic.all.order(Arel.sql("id ASC")) + topics = Topic.all.order("id ASC") assert_queries(1) do 2.times { assert_equal "The First Topic", topics.first.title } @@ -112,7 +112,7 @@ class RelationTest < ActiveRecord::TestCase end def test_loaded_first - topics = Topic.all.order(Arel.sql("id ASC")) + topics = Topic.all.order("id ASC") topics.load # force load assert_no_queries do @@ -123,7 +123,7 @@ class RelationTest < ActiveRecord::TestCase end def test_loaded_first_with_limit - topics = Topic.all.order(Arel.sql("id ASC")) + topics = Topic.all.order("id ASC") topics.load # force load assert_no_queries do @@ -135,7 +135,7 @@ class RelationTest < ActiveRecord::TestCase end def test_first_get_more_than_available - topics = Topic.all.order(Arel.sql("id ASC")) + topics = Topic.all.order("id ASC") unloaded_first = topics.first(10) topics.load # force load @@ -220,7 +220,7 @@ class RelationTest < ActiveRecord::TestCase end def test_finding_with_arel_assoc_order - topics = Topic.order(Arel.sql("id") => :desc) + topics = Topic.order("id" => :desc) assert_equal 5, topics.to_a.size assert_equal topics(:fifth).title, topics.first.title end @@ -232,7 +232,7 @@ class RelationTest < ActiveRecord::TestCase end def test_finding_with_reversed_arel_assoc_order - topics = Topic.order(Arel.sql("id") => :asc).reverse_order + topics = Topic.order("id" => :asc).reverse_order assert_equal 5, topics.to_a.size assert_equal topics(:fifth).title, topics.first.title end @@ -319,7 +319,7 @@ class RelationTest < ActiveRecord::TestCase end def test_raising_exception_on_invalid_hash_params - e = assert_raise(ArgumentError) { Topic.order(Arel.sql("name"), Arel.sql("id DESC"), id: :asfsdf) } + e = assert_raise(ArgumentError) { Topic.order(Arel.sql("name"), "id DESC", id: :asfsdf) } assert_equal 'Direction "asfsdf" is invalid. Valid directions are: [:asc, :desc, :ASC, :DESC, "asc", "desc", "ASC", "DESC"]', e.message end @@ -365,7 +365,7 @@ class RelationTest < ActiveRecord::TestCase end def test_finding_with_order_and_take - entrants = Entrant.order(Arel.sql("id ASC")).limit(2).to_a + entrants = Entrant.order("id ASC").limit(2).to_a assert_equal 2, entrants.size assert_equal entrants(:first).name, entrants.first.name @@ -374,7 +374,7 @@ class RelationTest < ActiveRecord::TestCase def test_finding_with_cross_table_order_and_limit tags = Tag.includes(:taggings). order( - Arel.sql("tags.name asc"), + "tags.name asc", Arel.sql("taggings.taggable_id asc"), Arel.sql("REPLACE('abc', taggings.taggable_type, taggings.taggable_type)") ).limit(1).to_a @@ -403,12 +403,12 @@ class RelationTest < ActiveRecord::TestCase end def test_finding_with_order_limit_and_offset - entrants = Entrant.order(Arel.sql("id ASC")).limit(2).offset(1) + entrants = Entrant.order("id ASC").limit(2).offset(1) assert_equal 2, entrants.to_a.size assert_equal entrants(:second).name, entrants.first.name - entrants = Entrant.order(Arel.sql("id ASC")).limit(2).offset(2) + entrants = Entrant.order("id ASC").limit(2).offset(2) assert_equal 1, entrants.to_a.size assert_equal entrants(:third).name, entrants.first.name end @@ -518,27 +518,27 @@ class RelationTest < ActiveRecord::TestCase def test_find_with_preloaded_associations assert_queries(2) do - posts = Post.preload(:comments).order(Arel.sql("posts.id")) + posts = Post.preload(:comments).order("posts.id") assert posts.first.comments.first end assert_queries(2) do - posts = Post.preload(:comments).order(Arel.sql("posts.id")) + posts = Post.preload(:comments).order("posts.id") assert posts.first.comments.first end assert_queries(2) do - posts = Post.preload(:author).order(Arel.sql("posts.id")) + posts = Post.preload(:author).order("posts.id") assert posts.first.author end assert_queries(2) do - posts = Post.preload(:author).order(Arel.sql("posts.id")) + posts = Post.preload(:author).order("posts.id") assert posts.first.author end assert_queries(3) do - posts = Post.preload(:author, :comments).order(Arel.sql("posts.id")) + posts = Post.preload(:author, :comments).order("posts.id") assert posts.first.author assert posts.first.comments.first end @@ -553,22 +553,22 @@ class RelationTest < ActiveRecord::TestCase def test_find_with_included_associations assert_queries(2) do - posts = Post.includes(:comments).order(Arel.sql("posts.id")) + posts = Post.includes(:comments).order("posts.id") assert posts.first.comments.first end assert_queries(2) do - posts = Post.all.includes(:comments).order(Arel.sql("posts.id")) + posts = Post.all.includes(:comments).order("posts.id") assert posts.first.comments.first end assert_queries(2) do - posts = Post.includes(:author).order(Arel.sql("posts.id")) + posts = Post.includes(:author).order("posts.id") assert posts.first.author end assert_queries(3) do - posts = Post.includes(:author, :comments).order(Arel.sql("posts.id")) + posts = Post.includes(:author, :comments).order("posts.id") assert posts.first.author assert posts.first.comments.first end @@ -698,7 +698,7 @@ class RelationTest < ActiveRecord::TestCase end def test_find_ids - authors = Author.order(Arel.sql("id ASC")) + authors = Author.order("id ASC") results = authors.find(authors(:david).id, authors(:mary).id) assert_kind_of Array, results @@ -982,7 +982,7 @@ class RelationTest < ActiveRecord::TestCase end def test_multiple_selects - post = Post.all.select("comments_count").select("title").order(Arel.sql("id ASC")).first + post = Post.all.select("comments_count").select("title").order("id ASC").first assert_equal "Welcome to the weblog", post.title assert_equal 2, post.comments_count end @@ -1344,7 +1344,7 @@ class RelationTest < ActiveRecord::TestCase end def test_except - relation = Post.where(author_id: 1).order(Arel.sql("id ASC")).limit(1) + relation = Post.where(author_id: 1).order("id ASC").limit(1) assert_equal [posts(:welcome)], relation.to_a author_posts = relation.except(:order, :limit) @@ -1355,7 +1355,7 @@ class RelationTest < ActiveRecord::TestCase end def test_only - relation = Post.where(author_id: 1).order(Arel.sql("id ASC")).limit(1) + relation = Post.where(author_id: 1).order("id ASC").limit(1) assert_equal [posts(:welcome)], relation.to_a author_posts = relation.only(:where) @@ -1366,7 +1366,7 @@ class RelationTest < ActiveRecord::TestCase end def test_anonymous_extension - relation = Post.where(author_id: 1).order(Arel.sql("id ASC")).extending do + relation = Post.where(author_id: 1).order("id ASC").extending do def author "lifo" end @@ -1377,7 +1377,7 @@ class RelationTest < ActiveRecord::TestCase end def test_named_extension - relation = Post.where(author_id: 1).order(Arel.sql("id ASC")).extending(Post::NamedExtension) + relation = Post.where(author_id: 1).order("id ASC").extending(Post::NamedExtension) assert_equal "lifo", relation.author assert_equal "lifo", relation.limit(1).author end @@ -1392,13 +1392,13 @@ class RelationTest < ActiveRecord::TestCase end def test_order_using_scoping - car1 = CoolCar.order(Arel.sql("id DESC")).scoping do - CoolCar.all.merge!(order: Arel.sql("id asc")).first + car1 = CoolCar.order("id DESC").scoping do + CoolCar.all.merge!(order: "id asc").first end assert_equal "zyke", car1.name - car2 = FastCar.order(Arel.sql("id DESC")).scoping do - FastCar.all.merge!(order: Arel.sql("id asc")).first + car2 = FastCar.order("id DESC").scoping do + FastCar.all.merge!(order: "id asc").first end assert_equal "zyke", car2.name end @@ -1442,7 +1442,7 @@ class RelationTest < ActiveRecord::TestCase end def test_update_all_with_joins_and_limit_and_order - comments = Comment.joins(:post).where("posts.id" => posts(:welcome).id).order(Arel.sql("comments.id")).limit(1) + comments = Comment.joins(:post).where("posts.id" => posts(:welcome).id).order("comments.id").limit(1) assert_equal 1, comments.update_all(post_id: posts(:thinking).id) assert_equal posts(:thinking), comments(:greetings).post assert_equal posts(:welcome), comments(:more_greetings).post @@ -1457,7 +1457,7 @@ class RelationTest < ActiveRecord::TestCase end def test_update_all_with_joins_and_offset_and_order - all_comments = Comment.joins(:post).where("posts.id" => posts(:welcome).id).order(Arel.sql("posts.id"), Arel.sql("comments.id")) + all_comments = Comment.joins(:post).where("posts.id" => posts(:welcome).id).order(Arel.sql("posts.id"), "comments.id") count = all_comments.count comments = all_comments.offset(1) @@ -1814,7 +1814,7 @@ class RelationTest < ActiveRecord::TestCase end test "joins with select" do - posts = Post.joins(:author).select("id", "authors.author_address_id").order(Arel.sql("posts.id")).limit(3) + posts = Post.joins(:author).select("id", "authors.author_address_id").order("posts.id").limit(3) assert_equal [1, 2, 4], posts.map(&:id) assert_equal [1, 1, 1], posts.map(&:author_address_id) end diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb index 26f0b86703..0d64db140c 100644 --- a/activerecord/test/cases/scoping/default_scoping_test.rb +++ b/activerecord/test/cases/scoping/default_scoping_test.rb @@ -13,7 +13,7 @@ class DefaultScopingTest < ActiveRecord::TestCase fixtures :developers, :posts, :comments def test_default_scope - expected = Developer.all.merge!(order: Arel.sql("salary DESC")).to_a.collect(&:salary) + expected = Developer.all.merge!(order: "salary DESC").to_a.collect(&:salary) received = DeveloperOrderedBySalary.all.collect(&:salary) assert_equal expected, received end @@ -86,14 +86,14 @@ class DefaultScopingTest < ActiveRecord::TestCase end def test_reorder_overrides_default_scope_order - expected = Developer.order(Arel.sql("name DESC")).collect(&:name) - received = DeveloperOrderedBySalary.reorder(Arel.sql("name DESC")).collect(&:name) + expected = Developer.order("name DESC").collect(&:name) + received = DeveloperOrderedBySalary.reorder("name DESC").collect(&:name) assert_equal expected, received end def test_order_after_reorder_combines_orders expected = Developer.order(Arel.sql("name DESC, id DESC")).collect { |dev| [dev.name, dev.id] } - received = Developer.order(Arel.sql("name ASC")).reorder(Arel.sql("name DESC")).order(Arel.sql("id DESC")).collect { |dev| [dev.name, dev.id] } + received = Developer.order("name ASC").reorder("name DESC").order("id DESC").collect { |dev| [dev.name, dev.id] } assert_equal expected, received end @@ -105,7 +105,7 @@ class DefaultScopingTest < ActiveRecord::TestCase def test_unscope_after_reordering_and_combining expected = Developer.order(Arel.sql("id DESC, name DESC")).collect { |dev| [dev.name, dev.id] } - received = DeveloperOrderedBySalary.reorder(Arel.sql("name DESC")).unscope(:order).order(Arel.sql("id DESC, name DESC")).collect { |dev| [dev.name, dev.id] } + received = DeveloperOrderedBySalary.reorder("name DESC").unscope(:order).order(Arel.sql("id DESC, name DESC")).collect { |dev| [dev.name, dev.id] } assert_equal expected, received expected_2 = Developer.all.collect { |dev| [dev.name, dev.id] } @@ -113,60 +113,60 @@ class DefaultScopingTest < ActiveRecord::TestCase assert_equal expected_2, received_2 expected_3 = Developer.all.collect { |dev| [dev.name, dev.id] } - received_3 = Developer.reorder(Arel.sql("name DESC")).unscope(:order).collect { |dev| [dev.name, dev.id] } + received_3 = Developer.reorder("name DESC").unscope(:order).collect { |dev| [dev.name, dev.id] } assert_equal expected_3, received_3 end def test_unscope_with_where_attributes - expected = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.where(name: "David").unscope(where: :name).collect(&:name) assert_equal expected, received - expected_2 = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected_2 = Developer.order("salary DESC").collect(&:name) received_2 = DeveloperOrderedBySalary.select("id").where("name" => "Jamis").unscope({ where: :name }, :select).collect(&:name) assert_equal expected_2, received_2 - expected_3 = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected_3 = Developer.order("salary DESC").collect(&:name) received_3 = DeveloperOrderedBySalary.select("id").where("name" => "Jamis").unscope(:select, :where).collect(&:name) assert_equal expected_3, received_3 - expected_4 = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected_4 = Developer.order("salary DESC").collect(&:name) received_4 = DeveloperOrderedBySalary.where.not("name" => "Jamis").unscope(where: :name).collect(&:name) assert_equal expected_4, received_4 - expected_5 = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected_5 = Developer.order("salary DESC").collect(&:name) received_5 = DeveloperOrderedBySalary.where.not("name" => ["Jamis", "David"]).unscope(where: :name).collect(&:name) assert_equal expected_5, received_5 - expected_6 = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected_6 = Developer.order("salary DESC").collect(&:name) received_6 = DeveloperOrderedBySalary.where(Developer.arel_table["name"].eq("David")).unscope(where: :name).collect(&:name) assert_equal expected_6, received_6 - expected_7 = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected_7 = Developer.order("salary DESC").collect(&:name) received_7 = DeveloperOrderedBySalary.where(Developer.arel_table[:name].eq("David")).unscope(where: :name).collect(&:name) assert_equal expected_7, received_7 end def test_unscope_comparison_where_clauses # unscoped for WHERE (`developers`.`id` <= 2) - expected = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.where(id: -Float::INFINITY..2).unscope(where: :id).collect { |dev| dev.name } assert_equal expected, received # unscoped for WHERE (`developers`.`id` < 2) - expected = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.where(id: -Float::INFINITY...2).unscope(where: :id).collect { |dev| dev.name } assert_equal expected, received end def test_unscope_multiple_where_clauses - expected = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.where(name: "Jamis").where(id: 1).unscope(where: [:name, :id]).collect(&:name) assert_equal expected, received end def test_unscope_string_where_clauses_involved - dev_relation = Developer.order(Arel.sql("salary DESC")).where("created_at > ?", 1.year.ago) + dev_relation = Developer.order("salary DESC").where("created_at > ?", 1.year.ago) expected = dev_relation.collect(&:name) dev_ordered_relation = DeveloperOrderedBySalary.where(name: "Jamis").where("created_at > ?", 1.year.ago) @@ -176,17 +176,17 @@ class DefaultScopingTest < ActiveRecord::TestCase end def test_unscope_with_grouping_attributes - expected = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.group(:name).unscope(:group).collect(&:name) assert_equal expected, received - expected_2 = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected_2 = Developer.order("salary DESC").collect(&:name) received_2 = DeveloperOrderedBySalary.group("name").unscope(:group).collect(&:name) assert_equal expected_2, received_2 end def test_unscope_with_limit_in_query - expected = Developer.order(Arel.sql("salary DESC")).collect(&:name) + expected = Developer.order("salary DESC").collect(&:name) received = DeveloperOrderedBySalary.limit(1).unscope(:limit).collect(&:name) assert_equal expected, received end @@ -198,13 +198,13 @@ class DefaultScopingTest < ActiveRecord::TestCase def test_unscope_reverse_order expected = Developer.all.collect(&:name) - received = Developer.order(Arel.sql("salary DESC")).reverse_order.unscope(:order).collect(&:name) + received = Developer.order("salary DESC").reverse_order.unscope(:order).collect(&:name) assert_equal expected, received end def test_unscope_select - expected = Developer.order(Arel.sql("salary ASC")).collect(&:name) - received = Developer.order(Arel.sql("salary DESC")).reverse_order.select(:name).unscope(:select).collect(&:name) + expected = Developer.order("salary ASC").collect(&:name) + received = Developer.order("salary DESC").reverse_order.select(:name).unscope(:select).collect(&:name) assert_equal expected, received expected_2 = Developer.all.collect(&:id) @@ -256,11 +256,11 @@ class DefaultScopingTest < ActiveRecord::TestCase end assert_raises(ArgumentError) do - Developer.order(Arel.sql("name DESC")).reverse_order.unscope(:reverse_order) + Developer.order("name DESC").reverse_order.unscope(:reverse_order) end assert_raises(ArgumentError) do - Developer.order(Arel.sql("name DESC")).where(name: "Jamis").unscope() + Developer.order("name DESC").where(name: "Jamis").unscope() end end @@ -295,7 +295,7 @@ class DefaultScopingTest < ActiveRecord::TestCase end def test_order_in_default_scope_should_not_prevail - expected = Developer.all.merge!(order: Arel.sql("salary desc")).to_a.collect(&:salary) + expected = Developer.all.merge!(order: "salary desc").to_a.collect(&:salary) received = DeveloperOrderedBySalary.all.merge!(order: "salary").to_a.collect(&:salary) assert_equal expected, received end diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb index 4079bb477e..b0431a4e34 100644 --- a/activerecord/test/cases/scoping/named_scoping_test.rb +++ b/activerecord/test/cases/scoping/named_scoping_test.rb @@ -472,7 +472,7 @@ class NamedScopingTest < ActiveRecord::TestCase def test_scopes_on_relations # Topic.replied - approved_topics = Topic.all.approved.order(Arel.sql("id DESC")) + approved_topics = Topic.all.approved.order("id DESC") assert_equal topics(:fifth), approved_topics.first replied_approved_topics = approved_topics.replied @@ -480,7 +480,7 @@ class NamedScopingTest < ActiveRecord::TestCase end def test_index_on_scope - approved = Topic.approved.order(Arel.sql("id ASC")) + approved = Topic.approved.order("id ASC") assert_equal topics(:second), approved[0] assert approved.loaded? end diff --git a/activerecord/test/cases/scoping/relation_scoping_test.rb b/activerecord/test/cases/scoping/relation_scoping_test.rb index 2a95204d0d..116f8e83aa 100644 --- a/activerecord/test/cases/scoping/relation_scoping_test.rb +++ b/activerecord/test/cases/scoping/relation_scoping_test.rb @@ -39,23 +39,23 @@ class RelationScopingTest < ActiveRecord::TestCase end def test_reverse_order - assert_equal Developer.order(Arel.sql("id DESC")).to_a.reverse, Developer.order(Arel.sql("id DESC")).reverse_order + assert_equal Developer.order("id DESC").to_a.reverse, Developer.order("id DESC").reverse_order end def test_reverse_order_with_arel_node - assert_equal Developer.order(Arel.sql("id DESC")).to_a.reverse, Developer.order(Developer.arel_table[:id].desc).reverse_order + assert_equal Developer.order("id DESC").to_a.reverse, Developer.order(Developer.arel_table[:id].desc).reverse_order end def test_reverse_order_with_multiple_arel_nodes - assert_equal Developer.order(Arel.sql("id DESC")).order(Arel.sql("name DESC")).to_a.reverse, Developer.order(Developer.arel_table[:id].desc).order(Developer.arel_table[:name].desc).reverse_order + assert_equal Developer.order("id DESC").order("name DESC").to_a.reverse, Developer.order(Developer.arel_table[:id].desc).order(Developer.arel_table[:name].desc).reverse_order end def test_reverse_order_with_arel_nodes_and_strings - assert_equal Developer.order(Arel.sql("id DESC")).order(Arel.sql("name DESC")).to_a.reverse, Developer.order(Arel.sql("id DESC")).order(Developer.arel_table[:name].desc).reverse_order + assert_equal Developer.order("id DESC").order("name DESC").to_a.reverse, Developer.order("id DESC").order(Developer.arel_table[:name].desc).reverse_order end def test_double_reverse_order_produces_original_order - assert_equal Developer.order(Arel.sql("name DESC")), Developer.order(Arel.sql("name DESC")).reverse_order.reverse_order + assert_equal Developer.order("name DESC"), Developer.order("name DESC").reverse_order.reverse_order end def test_scoped_find @@ -72,7 +72,7 @@ class RelationScopingTest < ActiveRecord::TestCase end def test_scoped_find_last - highest_salary = Developer.order(Arel.sql("salary DESC")).first + highest_salary = Developer.order("salary DESC").first Developer.order("salary").scoping do assert_equal highest_salary, Developer.last @@ -80,8 +80,8 @@ class RelationScopingTest < ActiveRecord::TestCase end def test_scoped_find_last_preserves_scope - lowest_salary = Developer.order(Arel.sql("salary ASC")).first - highest_salary = Developer.order(Arel.sql("salary DESC")).first + lowest_salary = Developer.order("salary ASC").first + highest_salary = Developer.order("salary DESC").first Developer.order("salary").scoping do assert_equal highest_salary, Developer.last diff --git a/activerecord/test/cases/unsafe_raw_sql_test.rb b/activerecord/test/cases/unsafe_raw_sql_test.rb index 74471b0ace..b32bb24ce8 100644 --- a/activerecord/test/cases/unsafe_raw_sql_test.rb +++ b/activerecord/test/cases/unsafe_raw_sql_test.rb @@ -77,10 +77,40 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase assert_equal ids_expected, ids_disabled end + test "order: allows table and column name" do + ids_expected = Post.order(Arel.sql("title")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order("posts.title").pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order("posts.title").pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows column name and direction in string" do + ids_expected = Post.order(Arel.sql("title desc")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order("title desc").pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order("title desc").pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + + test "order: allows table name, column name and direction in string" do + ids_expected = Post.order(Arel.sql("title desc")).pluck(:id) + + ids_depr = with_unsafe_raw_sql_deprecated { Post.order("posts.title desc").pluck(:id) } + ids_disabled = with_unsafe_raw_sql_disabled { Post.order("posts.title desc").pluck(:id) } + + assert_equal ids_expected, ids_depr + assert_equal ids_expected, ids_disabled + end + test "order: disallows invalid column name" do with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do - Post.order("title asc").pluck(:id) + Post.order("foo asc").pluck(:id) end end end @@ -168,6 +198,16 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase assert_equal values_expected, values_disabled end + test "pluck: allows table and column names" do + titles_expected = Post.pluck(Arel.sql("title")) + + titles_depr = with_unsafe_raw_sql_deprecated { Post.pluck("posts.title") } + titles_disabled = with_unsafe_raw_sql_disabled { Post.pluck("posts.title") } + + assert_equal titles_expected, titles_depr + assert_equal titles_expected, titles_disabled + end + test "pluck: disallows invalid column name" do with_unsafe_raw_sql_disabled do assert_raises(ActiveRecord::UnknownAttributeReference) do diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 11fe073ab0..f462fdb69a 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -8,9 +8,9 @@ class Author < ActiveRecord::Base has_many :posts_with_comments, -> { includes(:comments) }, class_name: "Post" has_many :popular_grouped_posts, -> { includes(:comments).group("type").having("SUM(comments_count) > 1").select("type") }, class_name: "Post" has_many :posts_with_comments_sorted_by_comment_id, -> { includes(:comments).order(Arel.sql("comments.id")) }, class_name: "Post" - has_many :posts_sorted_by_id_limited, -> { order(Arel.sql("posts.id")).limit(1) }, class_name: "Post" + has_many :posts_sorted_by_id_limited, -> { order("posts.id").limit(1) }, class_name: "Post" has_many :posts_with_categories, -> { includes(:categories) }, class_name: "Post" - has_many :posts_with_comments_and_categories, -> { includes(:comments, :categories).order(Arel.sql("posts.id")) }, class_name: "Post" + has_many :posts_with_comments_and_categories, -> { includes(:comments, :categories).order("posts.id") }, class_name: "Post" has_many :posts_with_special_categorizations, class_name: "PostWithSpecialCategorization" has_one :post_about_thinking, -> { where("posts.title like '%thinking%'") }, class_name: "Post" has_one :post_about_thinking_with_last_comment, -> { where("posts.title like '%thinking%'").includes(:last_comment) }, class_name: "Post" diff --git a/activerecord/test/models/car.rb b/activerecord/test/models/car.rb index 0be943a321..3d6a7a96c2 100644 --- a/activerecord/test/models/car.rb +++ b/activerecord/test/models/car.rb @@ -19,13 +19,13 @@ class Car < ActiveRecord::Base scope :incl_tyres, -> { includes(:tyres) } scope :incl_engines, -> { includes(:engines) } - scope :order_using_new_style, -> { order(Arel.sql("name asc")) } + scope :order_using_new_style, -> { order("name asc") } end class CoolCar < Car - default_scope { order(Arel.sql("name desc")) } + default_scope { order("name desc") } end class FastCar < Car - default_scope { order(Arel.sql("name desc")) } + default_scope { order("name desc") } end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 1a82a9e646..bbc5fc2b2d 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -50,7 +50,7 @@ class Firm < Company has_many :clients, -> { order "id" }, dependent: :destroy, before_remove: :log_before_remove, after_remove: :log_after_remove has_many :unsorted_clients, class_name: "Client" has_many :unsorted_clients_with_symbol, class_name: :Client - has_many :clients_sorted_desc, -> { order Arel.sql("id DESC") }, class_name: "Client" + has_many :clients_sorted_desc, -> { order "id DESC" }, class_name: "Client" has_many :clients_of_firm, -> { order "id" }, foreign_key: "client_of", class_name: "Client", inverse_of: :firm has_many :clients_ordered_by_name, -> { order "name" }, class_name: "Client" has_many :unvalidated_clients_of_firm, foreign_key: "client_of", class_name: "Client", validate: false diff --git a/activerecord/test/models/company_in_module.rb b/activerecord/test/models/company_in_module.rb index 9108eb5249..52b7e06a63 100644 --- a/activerecord/test/models/company_in_module.rb +++ b/activerecord/test/models/company_in_module.rb @@ -9,7 +9,7 @@ module MyApplication class Firm < Company has_many :clients, -> { order("id") }, dependent: :destroy - has_many :clients_sorted_desc, -> { order(Arel.sql("id DESC")) }, class_name: "Client" + has_many :clients_sorted_desc, -> { order("id DESC") }, class_name: "Client" has_many :clients_of_firm, -> { order "id" }, foreign_key: "client_of", class_name: "Client" has_many :clients_like_ms, -> { where("name = 'Microsoft'").order("id") }, class_name: "Client" has_one :account, class_name: "MyApplication::Billing::Account", dependent: :destroy diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index c2a21eac04..8881c69368 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -4,7 +4,7 @@ require "ostruct" module DeveloperProjectsAssociationExtension2 def find_least_recent - order(Arel.sql("id ASC")).first + order("id ASC").first end end @@ -13,7 +13,7 @@ class Developer < ActiveRecord::Base has_and_belongs_to_many :projects do def find_most_recent - order(Arel.sql("id DESC")).first + order("id DESC").first end end @@ -41,7 +41,7 @@ class Developer < ActiveRecord::Base join_table: "developers_projects", association_foreign_key: "project_id" do def find_least_recent - order(Arel.sql("id ASC")).first + order("id ASC").first end end @@ -126,7 +126,7 @@ end class DeveloperFilteredOnJoins < ActiveRecord::Base self.table_name = "developers" - has_and_belongs_to_many :projects, -> { order(Arel.sql("projects.id")) }, foreign_key: "developer_id", join_table: "developers_projects" + has_and_belongs_to_many :projects, -> { order("projects.id") }, foreign_key: "developer_id", join_table: "developers_projects" def self.default_scope joins(:projects).where(projects: { name: "Active Controller" }) @@ -135,9 +135,9 @@ end class DeveloperOrderedBySalary < ActiveRecord::Base self.table_name = "developers" - default_scope { order(Arel.sql("salary DESC")) } + default_scope { order("salary DESC") } - scope :by_name, -> { order(Arel.sql("name DESC")) } + scope :by_name, -> { order("name DESC") } end class DeveloperCalledDavid < ActiveRecord::Base @@ -225,14 +225,14 @@ end class EagerDeveloperWithDefaultScope < ActiveRecord::Base self.table_name = "developers" - has_and_belongs_to_many :projects, -> { order(Arel.sql("projects.id")) }, foreign_key: "developer_id", join_table: "developers_projects" + has_and_belongs_to_many :projects, -> { order("projects.id") }, foreign_key: "developer_id", join_table: "developers_projects" default_scope { includes(:projects) } end class EagerDeveloperWithClassMethodDefaultScope < ActiveRecord::Base self.table_name = "developers" - has_and_belongs_to_many :projects, -> { order(Arel.sql("projects.id")) }, foreign_key: "developer_id", join_table: "developers_projects" + has_and_belongs_to_many :projects, -> { order("projects.id") }, foreign_key: "developer_id", join_table: "developers_projects" def self.default_scope includes(:projects) @@ -241,21 +241,21 @@ end class EagerDeveloperWithLambdaDefaultScope < ActiveRecord::Base self.table_name = "developers" - has_and_belongs_to_many :projects, -> { order(Arel.sql("projects.id")) }, foreign_key: "developer_id", join_table: "developers_projects" + has_and_belongs_to_many :projects, -> { order("projects.id") }, foreign_key: "developer_id", join_table: "developers_projects" default_scope lambda { includes(:projects) } end class EagerDeveloperWithBlockDefaultScope < ActiveRecord::Base self.table_name = "developers" - has_and_belongs_to_many :projects, -> { order(Arel.sql("projects.id")) }, foreign_key: "developer_id", join_table: "developers_projects" + has_and_belongs_to_many :projects, -> { order("projects.id") }, foreign_key: "developer_id", join_table: "developers_projects" default_scope { includes(:projects) } end class EagerDeveloperWithCallableDefaultScope < ActiveRecord::Base self.table_name = "developers" - has_and_belongs_to_many :projects, -> { order(Arel.sql("projects.id")) }, foreign_key: "developer_id", join_table: "developers_projects" + has_and_belongs_to_many :projects, -> { order("projects.id") }, foreign_key: "developer_id", join_table: "developers_projects" default_scope OpenStruct.new(call: includes(:projects)) end diff --git a/activerecord/test/models/membership.rb b/activerecord/test/models/membership.rb index 47cb4d2146..09ee7544b3 100644 --- a/activerecord/test/models/membership.rb +++ b/activerecord/test/models/membership.rb @@ -12,7 +12,7 @@ class CurrentMembership < Membership end class SuperMembership < Membership - belongs_to :member, -> { order(Arel.sql("members.id DESC")) } + belongs_to :member, -> { order("members.id DESC") } belongs_to :club end diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index 99de37a52f..c8617d1cfe 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -3,7 +3,7 @@ class Pirate < ActiveRecord::Base belongs_to :parrot, validate: true belongs_to :non_validated_parrot, class_name: "Parrot" - has_and_belongs_to_many :parrots, -> { order(Arel.sql("parrots.id ASC")) }, validate: true + has_and_belongs_to_many :parrots, -> { order("parrots.id ASC") }, validate: true has_and_belongs_to_many :non_validated_parrots, class_name: "Parrot" has_and_belongs_to_many :parrots_with_method_callbacks, class_name: "Parrot", before_add: :log_before_add, @@ -23,7 +23,7 @@ class Pirate < ActiveRecord::Base has_one :ship has_one :update_only_ship, class_name: "Ship" has_one :non_validated_ship, class_name: "Ship" - has_many :birds, -> { order(Arel.sql("birds.id ASC")) } + has_many :birds, -> { order("birds.id ASC") } has_many :birds_with_method_callbacks, class_name: "Bird", before_add: :log_before_add, after_add: :log_after_add, diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 49bbbaaab7..4508f727d0 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -35,8 +35,8 @@ class Post < ActiveRecord::Base def first_comment super.body end - has_one :first_comment, -> { order(Arel.sql("id ASC")) }, class_name: "Comment" - has_one :last_comment, -> { order(Arel.sql("id desc")) }, class_name: "Comment" + has_one :first_comment, -> { order("id ASC") }, class_name: "Comment" + has_one :last_comment, -> { order("id desc") }, class_name: "Comment" scope :with_special_comments, -> { joins(:comments).where(comments: { type: "SpecialComment" }) } scope :with_very_special_comments, -> { joins(:comments).where(comments: { type: "VerySpecialComment" }) } @@ -52,7 +52,7 @@ class Post < ActiveRecord::Base has_many :comments do def find_most_recent - order(Arel.sql("id DESC")).first + order("id DESC").first end def newest @@ -323,5 +323,9 @@ class FakeKlass def enforce_raw_sql_whitelist(*args) # noop end + + def attribute_names_and_aliases + [] + end end end diff --git a/activerecord/test/models/tag.rb b/activerecord/test/models/tag.rb index e0d42f4f66..bc13c3a42d 100644 --- a/activerecord/test/models/tag.rb +++ b/activerecord/test/models/tag.rb @@ -11,6 +11,6 @@ end class OrderedTag < Tag self.table_name = "tags" - has_many :taggings, -> { order(Arel.sql("taggings.id DESC")) }, foreign_key: "tag_id" + has_many :taggings, -> { order("taggings.id DESC") }, foreign_key: "tag_id" has_many :tagged_posts, through: :taggings, source: "taggable", source_type: "Post" end |