Remove deprecated support to query using commas on LIMIT

Author: rafaelfranca

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Remove deprecated support to query using commas on LIMIT.
+
+    *Rafael Mendonça França*
+
 *   Remove deprecated support to passing a class as a value in a query.
 
     *Rafael Mendonça França*

activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb

@@ -334,17 +334,12 @@ def empty_insert_statement_value
       # Sanitizes the given LIMIT parameter in order to prevent SQL injection.
       #
       # The +limit+ may be anything that can evaluate to a string via #to_s. It
-      # should look like an integer, or a comma-delimited list of integers, or
-      # an Arel SQL literal.
+      # should look like an integer, or an Arel SQL literal.
       #
       # Returns Integer and Arel::Nodes::SqlLiteral limits as is.
-      # Returns the sanitized limit parameter, either as an integer, or as a
-      # string which contains a comma-delimited list of integers.
       def sanitize_limit(limit)
         if limit.is_a?(Integer) || limit.is_a?(Arel::Nodes::SqlLiteral)
           limit
-        elsif limit.to_s.include?(",")
-          Arel.sql limit.to_s.split(",").map { |i| Integer(i) }.join(",")
         else
           Integer(limit)
         end

activerecord/lib/active_record/relation/query_methods.rb

@@ -76,7 +76,7 @@ def #{method_name}=(value)           # def includes_values=(value)
     end
 
     def bound_attributes
-      if limit_value && !string_containing_comma?(limit_value)
+      if limit_value
         limit_bind = Attribute.with_cast_value(
           "LIMIT".freeze,
           connection.sanitize_limit(limit_value),
@@ -690,13 +690,6 @@ def limit(value)
     end
 
     def limit!(value) # :nodoc:
-      if string_containing_comma?(value)
-        # Remove `string_containing_comma?` when removing this deprecation
-        ActiveSupport::Deprecation.warn(<<-WARNING.squish)
-          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.
-        WARNING
-      end
       self.limit_value = value
       self
     end
@@ -958,13 +951,7 @@ def build_arel
 
         arel.where(where_clause.ast) unless where_clause.empty?
         arel.having(having_clause.ast) unless having_clause.empty?
-        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.take(Arel::Nodes::BindParam.new) if limit_value
         arel.skip(Arel::Nodes::BindParam.new) if offset_value
         arel.group(*arel_columns(group_values.uniq.reject(&:blank?))) unless group_values.empty?
 
@@ -1192,10 +1179,6 @@ def where_clause_factory
       end
       alias having_clause_factory where_clause_factory
 
-      def string_containing_comma?(value)
-        ::String === value && value.include?(",")
-      end
-
       def default_value_for(name)
         case name
         when :create_with

activerecord/test/cases/base_test.rb

@@ -107,14 +107,6 @@ def test_primary_key_with_no_id
     assert_nil Edge.primary_key
   end
 
-  unless current_adapter?(:PostgreSQLAdapter, :OracleAdapter, :SQLServerAdapter, :FbAdapter)
-    def test_limit_with_comma
-      assert_deprecated do
-        assert Topic.limit("1,2").to_a
-      end
-    end
-  end
-
   def test_many_mutations
     car = Car.new name: "<3<3<3"
     car.engines_count = 0
@@ -144,10 +136,8 @@ def test_limit_should_sanitize_sql_injection_for_limit_without_commas
   end
 
   def test_limit_should_sanitize_sql_injection_for_limit_with_commas
-    assert_deprecated do
-      assert_raises(ArgumentError) do
-        Topic.limit("1, 7 procedure help()").to_a
-      end
+    assert_raises(ArgumentError) do
+      Topic.limit("1, 7 procedure help()").to_a
     end
   end