diff options
Diffstat (limited to 'activerecord')
| -rw-r--r-- | activerecord/CHANGELOG.md | 9 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/query_methods.rb | 34 | ||||
| -rw-r--r-- | activerecord/test/cases/base_test.rb | 6 | ||||
| -rw-r--r-- | activerecord/test/cases/finder_test.rb | 6 |
4 files changed, 42 insertions, 13 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 56a3232ee9..8cedfd5277 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Use bind params for `limit` and `offset`. This will generate significantly + fewer prepared statements for common tasks like pagination. To support this + change, passing a string containing a comma to `limit` has been deprecated, + and passing an Arel node to `limit` is no longer supported. + + Fixes #22250 + + *Sean Griffin* + * Introduce after_{create,update,delete}_commit callbacks. Before: diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index f7115c7a91..66f00c31e2 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -97,7 +97,22 @@ module ActiveRecord end def bound_attributes - from_clause.binds + arel.bind_values + where_clause.binds + having_clause.binds + result = from_clause.binds + arel.bind_values + where_clause.binds + having_clause.binds + if limit_value && !string_containing_comma?(limit_value) + result << Attribute.with_cast_value( + "LIMIT".freeze, + connection.sanitize_limit(limit_value), + Type::Value.new, + ) + end + if offset_value + result << Attribute.with_cast_value( + "OFFSET".freeze, + offset_value.to_i, + Type::Value.new, + ) + end + result end def create_with_value # :nodoc: @@ -677,7 +692,8 @@ module ActiveRecord end def limit!(value) # :nodoc: - if ::String === value && value.include?(",") + if string_containing_comma?(value) + # Remove `string_containing_comma?` when removing this deprecation ActiveSupport::Deprecation.warn(<<-WARNING) Passing a string to limit in the form "1,2" is deprecated and will be removed in Rails 5.1. Please call `offset` explicitly instead. @@ -933,8 +949,14 @@ module ActiveRecord arel.where(where_clause.ast) unless where_clause.empty? arel.having(having_clause.ast) unless having_clause.empty? - arel.take(connection.sanitize_limit(limit_value)) if limit_value - arel.skip(offset_value.to_i) if offset_value + if limit_value + if string_containing_comma?(limit_value) + arel.take(connection.sanitize_limit(limit_value)) + else + arel.take(Arel::Nodes::BindParam.new) + end + end + arel.skip(Arel::Nodes::BindParam.new) if offset_value arel.group(*arel_columns(group_values.uniq.reject(&:blank?))) unless group_values.empty? build_order(arel) @@ -1183,5 +1205,9 @@ module ActiveRecord def new_from_clause Relation::FromClause.empty end + + def string_containing_comma?(value) + ::String === value && value.include?(",") + end end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index b449280fb4..dc555caaff 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -147,12 +147,6 @@ class BasicsTest < ActiveRecord::TestCase end end - unless current_adapter?(:MysqlAdapter, :Mysql2Adapter) - def test_limit_should_allow_sql_literal - assert_equal 1, Topic.limit(Arel.sql('2-1')).to_a.length - end - end - def test_select_symbol topic_ids = Topic.select(:id).map(&:id).sort assert_equal Topic.pluck(:id).sort, topic_ids diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 91214da048..73f5312eba 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -434,9 +434,9 @@ class FinderTest < ActiveRecord::TestCase end def test_take_and_first_and_last_with_integer_should_use_sql_limit - assert_sql(/LIMIT 3|ROWNUM <= 3/) { Topic.take(3).entries } - assert_sql(/LIMIT 2|ROWNUM <= 2/) { Topic.first(2).entries } - assert_sql(/LIMIT 5|ROWNUM <= 5/) { Topic.last(5).entries } + assert_sql(/LIMIT|ROWNUM <=/) { Topic.take(3).entries } + assert_sql(/LIMIT|ROWNUM <=/) { Topic.first(2).entries } + assert_sql(/LIMIT|ROWNUM <=/) { Topic.last(5).entries } end def test_last_with_integer_and_order_should_keep_the_order |