diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b4870cdbc5489..55b58ea67a38b 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Deprecate `in_clause_length` in `DatabaseLimits`. + + *Ryuta Kamizono* + * Fix aggregate functions to return numeric value consistently even on custom attribute type. *Ryuta Kamizono* diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb index d932f068f2f8a..e4ba741673ce5 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb @@ -61,6 +61,7 @@ def columns_per_multicolumn_index def in_clause_length nil end + deprecate :in_clause_length # Returns the maximum length of an SQL query. def sql_query_length diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb index eff7a0d03695b..987beaa2d3681 100644 --- a/activerecord/lib/arel/visitors/to_sql.rb +++ b/activerecord/lib/arel/visitors/to_sql.rb @@ -513,64 +513,33 @@ def visit_Arel_Table(o, collector) end def visit_Arel_Nodes_In(o, collector) - unless Array === o.right - return collect_in_clause(o.left, o.right, collector) - end - - unless o.right.empty? - o.right.delete_if { |value| unboundable?(value) } - end - - return collector << "1=0" if o.right.empty? - - in_clause_length = @connection.in_clause_length + attr, values = o.left, o.right - if !in_clause_length || o.right.length <= in_clause_length - collect_in_clause(o.left, o.right, collector) - else - collector << "(" - o.right.each_slice(in_clause_length).each_with_index do |right, i| - collector << " OR " unless i == 0 - collect_in_clause(o.left, right, collector) + if Array === values + unless values.empty? + values.delete_if { |value| unboundable?(value) } end - collector << ")" + + return collector << "1=0" if values.empty? end - end - def collect_in_clause(left, right, collector) - collector = visit left, collector - collector << " IN (" - visit(right, collector) << ")" + visit(attr, collector) << " IN (" + visit(values, collector) << ")" end def visit_Arel_Nodes_NotIn(o, collector) - unless Array === o.right - return collect_not_in_clause(o.left, o.right, collector) - end - - unless o.right.empty? - o.right.delete_if { |value| unboundable?(value) } - end - - return collector << "1=1" if o.right.empty? - - in_clause_length = @connection.in_clause_length + attr, values = o.left, o.right - if !in_clause_length || o.right.length <= in_clause_length - collect_not_in_clause(o.left, o.right, collector) - else - o.right.each_slice(in_clause_length).each_with_index do |right, i| - collector << " AND " unless i == 0 - collect_not_in_clause(o.left, right, collector) + if Array === values + unless values.empty? + values.delete_if { |value| unboundable?(value) } end - collector + + return collector << "1=1" if values.empty? end - end - def collect_not_in_clause(left, right, collector) - collector = visit left, collector - collector << " NOT IN (" - visit(right, collector) << ")" + visit(attr, collector) << " NOT IN (" + visit(values, collector) << ")" end def visit_Arel_Nodes_And(o, collector) diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 9eb8ec300eb9f..34f1a7de98f3d 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -410,6 +410,12 @@ def test_sql_query_length_is_deprecated def test_joins_per_query_is_deprecated assert_deprecated { @connection.joins_per_query } end + + unless current_adapter?(:OracleAdapter) + def test_in_clause_length_is_deprecated + assert_deprecated { @connection.in_clause_length } + end + end end class AdapterForeignKeyTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/arel/support/fake_record.rb b/activerecord/test/cases/arel/support/fake_record.rb index f03c965232833..864f47e245f51 100644 --- a/activerecord/test/cases/arel/support/fake_record.rb +++ b/activerecord/test/cases/arel/support/fake_record.rb @@ -64,10 +64,6 @@ def sanitize_as_sql_comment(comment) comment end - def in_clause_length - 3 - end - def schema_cache self end diff --git a/activerecord/test/cases/arel/visitors/to_sql_test.rb b/activerecord/test/cases/arel/visitors/to_sql_test.rb index 2a44572f2ab88..8fcf600b1328b 100644 --- a/activerecord/test/cases/arel/visitors/to_sql_test.rb +++ b/activerecord/test/cases/arel/visitors/to_sql_test.rb @@ -395,11 +395,6 @@ def dispatch _(compile(node)).must_be_like %{ "users"."id" IN (1, 2, 3) } - - node = @attr.in [1, 2, 3, 4, 5] - _(compile(node)).must_be_like %{ - ("users"."id" IN (1, 2, 3) OR "users"."id" IN (4, 5)) - } end it "should return 1=0 when empty right which is always false" do @@ -550,11 +545,6 @@ def dispatch _(compile(node)).must_be_like %{ "users"."id" NOT IN (1, 2, 3) } - - node = @attr.not_in [1, 2, 3, 4, 5] - _(compile(node)).must_be_like %{ - "users"."id" NOT IN (1, 2, 3) AND "users"."id" NOT IN (4, 5) - } end it "should return 1=1 when empty right which is always true" do diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 015423249fc32..1b883c7838c67 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -222,61 +222,6 @@ def test_duplicate_middle_objects end end - def test_preloading_has_many_in_multiple_queries_with_more_ids_than_database_can_handle - assert_called(Comment.connection, :in_clause_length, returns: 5) do - posts = Post.all.merge!(includes: :comments).to_a - assert_equal 11, posts.size - end - end - - def test_preloading_has_many_in_one_queries_when_database_has_no_limit_on_ids_it_can_handle - assert_called(Comment.connection, :in_clause_length, returns: nil) do - posts = Post.all.merge!(includes: :comments).to_a - assert_equal 11, posts.size - end - end - - def test_preloading_habtm_in_multiple_queries_with_more_ids_than_database_can_handle - assert_called(Comment.connection, :in_clause_length, times: 2, returns: 5) do - posts = Post.all.merge!(includes: :categories).to_a - assert_equal 11, posts.size - end - end - - def test_preloading_habtm_in_one_queries_when_database_has_no_limit_on_ids_it_can_handle - assert_called(Comment.connection, :in_clause_length, times: 2, returns: nil) do - posts = Post.all.merge!(includes: :categories).to_a - assert_equal 11, posts.size - end - end - - def test_load_associated_records_in_one_query_when_adapter_has_no_limit - assert_not_called(Comment.connection, :in_clause_length) do - post = posts(:welcome) - assert_queries(2) do - Post.includes(:comments).where(id: post.id).to_a - end - end - end - - def test_load_associated_records_in_several_queries_when_many_ids_passed - assert_called(Comment.connection, :in_clause_length, times: 2, returns: 1) do - post1, post2 = posts(:welcome), posts(:thinking) - assert_queries(2) do - Post.includes(:comments).where(id: [post1.id, post2.id]).to_a - end - end - end - - def test_load_associated_records_in_one_query_when_a_few_ids_passed - assert_not_called(Comment.connection, :in_clause_length) do - post = posts(:welcome) - assert_queries(2) do - Post.includes(:comments).where(id: post.id).to_a - end - end - end - def test_including_duplicate_objects_from_belongs_to popular_post = Post.create!(title: "foo", body: "I like cars!") comment = popular_post.comments.create!(body: "lol") diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb index 9918205b67d84..3a1facc318ddf 100644 --- a/activerecord/test/cases/relation/where_test.rb +++ b/activerecord/test/cases/relation/where_test.rb @@ -20,13 +20,6 @@ module ActiveRecord class WhereTest < ActiveRecord::TestCase fixtures :posts, :comments, :edges, :authors, :author_addresses, :binaries, :essays, :cars, :treasures, :price_estimates, :topics - def test_in_clause_is_correctly_sliced - assert_called(Author.connection, :in_clause_length, returns: 1) do - david = authors(:david) - assert_equal [david], Author.where(name: "David", id: [1, 2]) - end - end - def test_type_casting_nested_joins comment = comments(:eager_other_comment1) assert_equal [comment], Comment.joins(post: :author).where(authors: { id: "2-foo" })