Deprecate limit strings with commas

Some backends allow `LIMIT 1,2` as a shorthand for `LIMIT 1 OFFSET 2`.
Supporting this in Active Record massively complicates using bind
parameters for limit and offset, and it's trivially easy to build an
invalid SQL query by also calling `offset` on the same `Relation`.

This is a niche syntax that is only supported by a few adapters, and can
be trivially worked around by calling offset explicitly.

2 files changed, 13 insertions, 3 deletions

diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index dbecb842b5..f7115c7a91 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -677,6 +677,12 @@ module ActiveRecord
     end
 
     def limit!(value) # :nodoc:
+      if ::String === value && value.include?(",")
+        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.
+        WARNING
+      end
       self.limit_value = value
       self
     end
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 3a9d60a79f..b449280fb4 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -112,7 +112,9 @@ class BasicsTest < ActiveRecord::TestCase
 
   unless current_adapter?(:PostgreSQLAdapter, :OracleAdapter, :SQLServerAdapter, :FbAdapter)
     def test_limit_with_comma
-      assert Topic.limit("1,2").to_a
+      assert_deprecated do
+        assert Topic.limit("1,2").to_a
+      end
     end
   end
 
@@ -138,8 +140,10 @@ class BasicsTest < ActiveRecord::TestCase
   end
 
   def test_limit_should_sanitize_sql_injection_for_limit_with_commas
-    assert_raises(ArgumentError) do
-      Topic.limit("1, 7 procedure help()").to_a
+    assert_deprecated do
+      assert_raises(ArgumentError) do
+        Topic.limit("1, 7 procedure help()").to_a
+      end
     end
   end