Use a bind param for LIMIT and OFFSET

We currently generate an unbounded number of prepared statements when
`limit` or `offset` are called with a dynamic argument. This changes
`LIMIT` and `OFFSET` to use bind params, eliminating the problem.

`Type::Value#hash` needed to be implemented, as it turns out we busted
the query cache if the type object used wasn't exactly the same object.

This drops support for passing an `Arel::Nodes::SqlLiteral` to `limit`.
Doing this relied on AR internals, and was never officially supported
usage.

Fixes #22250.

activemodel/lib/active_model/type/value.rb

@@ -90,6 +90,11 @@ module ActiveModel
           scale == other.scale &&
           limit == other.limit
       end
+      alias eql? ==
+
+      def hash
+        [self.class, precision, scale, limit].hash
+      end
 
       def assert_valid_value(*)
       end

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:

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

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

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