allow Arel.sql() for pluck

8 files changed, 159 insertions, 101 deletions

diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index fa0d79ba5f..b3d3c0559f 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -167,6 +167,30 @@ module ActiveRecord
         end
       end
 
+      def enforce_raw_sql_whitelist(args, whitelist: attribute_names_and_aliases) # :nodoc:
+        return if allow_unsafe_raw_sql == :enabled
+
+        unexpected = args.reject do |arg|
+          whitelist.include?(arg.to_s) ||
+            arg.kind_of?(Arel::Node) || arg.is_a?(Arel::Nodes::SqlLiteral)
+        end
+
+        return if unexpected.none?
+
+        if allow_unsafe_raw_sql == :deprecated
+          ActiveSupport::Deprecation.warn(
+            "Dangerous query method used with non-attribute argument(s): " +
+            "#{unexpected.map(&:inspect).join(", ")}. Non-argument " +
+            "arguments will be disallowed in Rails 5.3."
+          )
+        else
+          raise(ActiveRecord::UnknownAttributeReference,
+            "Query method called with non-attribute argument(s): " +
+            unexpected.map(&:inspect).join(", ")
+          )
+        end
+      end
+
       # Can the given name be treated as a column name? Returns true if name
       # is attribute or attribute alias.
       #
@@ -178,7 +202,7 @@ module ActiveRecord
       #
       #   Person.respond_to_attribute?("foo")
       #   # => false
-      def respond_to_attribute?(name)
+      def respond_to_attribute?(name) # :nodoc:
         name = name.to_s
         attribute_names.include?(name) || attribute_aliases.include?(name)
       end
@@ -214,6 +238,18 @@ module ActiveRecord
           ConnectionAdapters::NullColumn.new(name)
         end
       end
+
+      # An Array of String attribute names and aliases for accessing those
+      # attributes.
+      #
+      #   class Person < ActiveRecord::Base
+      #   end
+      #
+      #   Person.attribute_names_and_aliases
+      #   # => ["id", "created_at", "updated_at", "name", "age"]
+      def attribute_names_and_aliases # :nodoc:
+        attribute_names + attribute_aliases.keys
+      end
     end
 
     # A Person object with a name attribute can ask <tt>person.respond_to?(:name)</tt>,
diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb
index 9ef3316393..3c039eef82 100644
--- a/activerecord/lib/active_record/errors.rb
+++ b/activerecord/lib/active_record/errors.rb
@@ -339,4 +339,29 @@ module ActiveRecord
   # Wait time value is set by innodb_lock_wait_timeout.
   class TransactionTimeout < StatementInvalid
   end
+
+  # UnknownAttributeReference is raised when an unknown and potentially unsafe
+  # value is passed to a query method when allow_unsafe_raw_sql is set to
+  # :disabled. For example, passing a non column name value to a relation's
+  # #order method might cause this exception.
+  #
+  # When working around this exception, caution should be taken to avoid SQL
+  # injection vulnerabilities when passing user-provided values to query
+  # methods. Known-safe values can be passed to query methods by wrapping them
+  # in Arel.sql.
+  #
+  # For example, with allow_unsafe_raw_sql set to :disabled, the following
+  # code would raise this exception:
+  #
+  #   Post.order("length(title)").first
+  #
+  # The desired result can be accomplished by wrapping the known-safe string
+  # in Arel.sql:
+  #
+  #   Post.order(Arel.sql("length(title)")).first
+  #
+  # Again, such a workaround should *not* be used when passing user-provided
+  # values, such as request parameters or model attributes to query methods.
+  class UnknownAttributeReference < ActiveRecordError
+  end
 end
diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb
index 0233835bcf..3996d5661f 100644
--- a/activerecord/lib/active_record/querying.rb
+++ b/activerecord/lib/active_record/querying.rb
@@ -2,25 +2,18 @@
 
 module ActiveRecord
   module Querying
-    delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?,
-             :any?, :many?, :none?, :one?, to: :all
-    delegate :second, :second!, :third, :third!, :fourth, :fourth!, :fifth,
-             :fifth!, :forty_two, :forty_two!, :third_to_last, :third_to_last!,
-             :second_to_last, :second_to_last!, to: :all
+    delegate :find, :take, :take!, :first, :first!, :last, :last!, :exists?, :any?, :many?, :none?, :one?, to: :all
+    delegate :second, :second!, :third, :third!, :fourth, :fourth!, :fifth, :fifth!, :forty_two, :forty_two!, :third_to_last, :third_to_last!, :second_to_last, :second_to_last!, to: :all
     delegate :first_or_create, :first_or_create!, :first_or_initialize, to: :all
-    delegate :find_or_create_by, :find_or_create_by!, :find_or_initialize_by,
-             to: :all
+    delegate :find_or_create_by, :find_or_create_by!, :find_or_initialize_by, to: :all
     delegate :find_by, :find_by!, to: :all
     delegate :destroy_all, :delete_all, :update_all, to: :all
     delegate :find_each, :find_in_batches, :in_batches, to: :all
-    delegate :select, :group, :order, :unsafe_raw_order, :except, :reorder,
-             :unsafe_raw_reorder, :limit, :offset, :joins, :left_joins,
-             :left_outer_joins, :or, :where, :rewhere, :preload, :eager_load,
-             :includes, :from, :lock, :readonly, :extending, :having,
-             :create_with, :distinct, :references, :none, :unscope, :merge,
-             to: :all
+    delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :left_joins, :left_outer_joins, :or,
+             :where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly, :extending,
+             :having, :create_with, :distinct, :references, :none, :unscope, :merge, to: :all
     delegate :count, :average, :minimum, :maximum, :sum, :calculate, to: :all
-    delegate :pluck, :unsafe_raw_pluck, :ids, to: :all
+    delegate :pluck, :ids, to: :all
 
     # Executes a custom SQL query against your database and returns all the results. The results will
     # be returned as an array with columns requested encapsulated as attributes of the model you call
diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb
index dea7542f03..236d36e15f 100644
--- a/activerecord/lib/active_record/relation/calculations.rb
+++ b/activerecord/lib/active_record/relation/calculations.rb
@@ -175,13 +175,21 @@ module ActiveRecord
     # See also #ids.
     #
     def pluck(*column_names)
-      _pluck(column_names, @klass.allow_unsafe_raw_sql == :enabled)
-    end
+      if loaded? && (column_names.map(&:to_s) - @klass.attribute_names_and_aliases).empty?
+        return records.pluck(*column_names)
+      end
 
-    # Same as #pluck but allows raw SQL regardless of `allow_unsafe_raw_sql`
-    # config setting.
-    def unsafe_raw_pluck(*column_names)
-      _pluck(column_names, true)
+      if has_include?(column_names.first)
+        construct_relation_for_association_calculations.pluck(*column_names)
+      else
+        enforce_raw_sql_whitelist(column_names)
+        relation = spawn
+        relation.select_values = column_names.map { |cn|
+          @klass.respond_to_attribute?(cn) ? arel_attribute(cn) : cn
+        }
+        result = klass.connection.select_all(relation.arel, nil, bound_attributes)
+        result.cast_values(klass.attribute_types)
+      end
     end
 
     # Pluck all the ID's for the relation using the table's primary key
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 63b1d8e154..4c63d0450a 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -295,22 +295,9 @@ module ActiveRecord
       spawn.order!(*args)
     end
 
-    # Same as #order but allows raw SQL regardless of `allow_unsafe_raw_sql`
-    # config setting.
-    def unsafe_raw_order(*args) # :nodoc:
-      check_if_method_has_arguments!(:order, args)
-      spawn.unsafe_raw_order!(*args)
-    end
-
     # Same as #order but operates on relation in-place instead of copying.
     def order!(*args) # :nodoc:
-      restrict_order_args(args) unless klass.allow_unsafe_raw_sql == :enabled
-      unsafe_raw_order!(*args)
-    end
-
-    # Same as #order! but allows raw SQL regardless of `allow_unsafe_raw_sql`
-    # config setting.
-    def unsafe_raw_order!(*args) # :nodoc:
+      enforce_raw_sql_whitelist(column_names_from_order_arguments(args))
       preprocess_order_args(args)
 
       self.order_values += args
@@ -331,22 +318,9 @@ module ActiveRecord
       spawn.reorder!(*args)
     end
 
-    # Same as #reorder but allows raw SQL regardless of `allow_unsafe_raw_sql`
-    # config setting.
-    def unsafe_raw_reorder(*args) # :nodoc:
-      check_if_method_has_arguments!(:reorder, args)
-      spawn.unsafe_raw_reorder!(*args)
-    end
-
     # Same as #reorder but operates on relation in-place instead of copying.
     def reorder!(*args) # :nodoc:
-      restrict_order_args(args) unless klass.allow_unsafe_raw_sql == :enabled
-      unsafe_raw_reorder!
-    end
-
-    # Same as #reorder! but allows raw SQL regardless of `allow_unsafe_raw_sql`
-    # config setting.
-    def unsafe_raw_reorder!(*args) # :nodoc:
+      enforce_raw_sql_whitelist(column_names_from_order_arguments(args))
       preprocess_order_args(args)
 
       self.reordering_value = true
@@ -946,6 +920,11 @@ module ActiveRecord
 
     private
 
+      # Extract column names from arguments passed to #order or #reorder.
+      def column_names_from_order_arguments(args)
+        args.flat_map { |arg| arg.is_a?(Hash) ? arg.keys : arg }
+      end
+
       def assert_mutability!
         raise ImmutableRelation if @loaded
         raise ImmutableRelation if defined?(@arel) && @arel
@@ -1169,25 +1148,6 @@ module ActiveRecord
         end.flatten!
       end
 
-      # Only allow column names and directions as arguments to #order and
-      # #reorder. Other arguments will cause an ArugmentError to be raised.
-      def restrict_order_args(args)
-        args = args.dup
-        orderings = args.extract_options!
-        columns = args | orderings.keys
-
-        unrecognized = columns.reject { |c| klass.respond_to_attribute?(c) }
-        if unrecognized.any?
-          raise ArgumentError, "Invalid order column: #{unrecognized}"
-        end
-
-        # TODO: find a better list of modifiers.
-        unrecognized = orderings.values.reject { |d| VALID_DIRECTIONS.include?(d.to_s) }
-        if unrecognized.any?
-          raise ArgumentError, "Invalid order direction: #{unrecognized}"
-        end
-      end
-
       # Checks to make sure that the arguments are not blank. Note that if some
       # blank-like object were initially passed into the query method, then this
       # method will not raise an error.
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index 66bc14b5ab..20dcb0080b 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -144,7 +144,7 @@ class CalculationsTest < ActiveRecord::TestCase
   end
 
   def test_should_order_by_calculation
-    c = Account.group(:firm_id).order("sum_credit_limit desc, firm_id").sum(:credit_limit)
+    c = Account.group(:firm_id).order(Arel.sql("sum_credit_limit desc, firm_id")).sum(:credit_limit)
     assert_equal [105, 60, 53, 50, 50], c.keys.collect { |k| c[k] }
     assert_equal [6, 2, 9, 1], c.keys.compact
   end
@@ -644,7 +644,7 @@ class CalculationsTest < ActiveRecord::TestCase
   end
 
   def test_pluck_with_qualified_column_name
-    assert_equal [1, 2, 3, 4, 5], Topic.order(:id).pluck("topics.id")
+    assert_equal [1, 2, 3, 4, 5], Topic.order(:id).pluck(Arel.sql("topics.id"))
   end
 
   def test_pluck_auto_table_name_prefix
@@ -659,18 +659,18 @@ class CalculationsTest < ActiveRecord::TestCase
 
   def test_pluck_not_auto_table_name_prefix_if_column_joined
     Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)])
-    assert_equal [7], Company.joins(:contracts).pluck(:developer_id)
+    assert_equal [7], Company.joins(:contracts).pluck(Arel.sql("developer_id"))
   end
 
   def test_pluck_with_selection_clause
-    assert_equal [50, 53, 55, 60], Account.pluck("DISTINCT credit_limit").sort
-    assert_equal [50, 53, 55, 60], Account.pluck("DISTINCT accounts.credit_limit").sort
-    assert_equal [50, 53, 55, 60], Account.pluck("DISTINCT(credit_limit)").sort
+    assert_equal [50, 53, 55, 60], Account.pluck(Arel.sql("DISTINCT credit_limit")).sort
+    assert_equal [50, 53, 55, 60], Account.pluck(Arel.sql("DISTINCT accounts.credit_limit")).sort
+    assert_equal [50, 53, 55, 60], Account.pluck(Arel.sql("DISTINCT(credit_limit)")).sort
 
     # MySQL returns "SUM(DISTINCT(credit_limit))" as the column name unless
     # an alias is provided.  Without the alias, the column cannot be found
     # and properly typecast.
-    assert_equal [50 + 53 + 55 + 60], Account.pluck("SUM(DISTINCT(credit_limit)) as credit_limit")
+    assert_equal [50 + 53 + 55 + 60], Account.pluck(Arel.sql("SUM(DISTINCT(credit_limit)) as credit_limit"))
   end
 
   def test_plucks_with_ids
@@ -684,7 +684,7 @@ class CalculationsTest < ActiveRecord::TestCase
 
   def test_pluck_not_auto_table_name_prefix_if_column_included
     Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)])
-    ids = Company.includes(:contracts).pluck(:developer_id)
+    ids = Company.includes(:contracts).pluck(Arel.sql("developer_id"))
     assert_equal Company.count, ids.length
     assert_equal [7], ids.compact
   end
@@ -704,12 +704,12 @@ class CalculationsTest < ActiveRecord::TestCase
 
   def test_pluck_with_multiple_columns_and_selection_clause
     assert_equal [[1, 50], [2, 50], [3, 50], [4, 60], [5, 55], [6, 53]],
-      Account.pluck("id, credit_limit")
+      Account.pluck(Arel.sql("id, credit_limit"))
   end
 
   def test_pluck_with_multiple_columns_and_includes
     Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)])
-    companies_and_developers = Company.order("companies.id").includes(:contracts).pluck(:name, :developer_id)
+    companies_and_developers = Company.order(Arel.sql("companies.id")).includes(:contracts).pluck(:name, Arel.sql("developer_id"))
 
     assert_equal Company.count, companies_and_developers.length
     assert_equal ["37signals", nil], companies_and_developers.first
@@ -731,7 +731,7 @@ class CalculationsTest < ActiveRecord::TestCase
   def test_pluck_columns_with_same_name
     expected = [["The First Topic", "The Second Topic of the day"], ["The Third Topic of the day", "The Fourth Topic of the day"]]
     actual = Topic.joins(:replies)
-      .pluck("topics.title", "replies_topics.title")
+      .pluck(Arel.sql("topics.title"), Arel.sql("replies_topics.title"))
     assert_equal expected, actual
   end
 
@@ -772,7 +772,7 @@ class CalculationsTest < ActiveRecord::TestCase
     companies = Company.order(:name).limit(3).load
 
     assert_queries 1 do
-      assert_equal ["37signals", "Apex", "Ex Nihilo"], companies.pluck("DISTINCT name")
+      assert_equal ["37signals", "Apex", "Ex Nihilo"], companies.pluck(Arel.sql("DISTINCT name"))
     end
   end
 
diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb
index b68b3723f6..953e0fee76 100644
--- a/activerecord/test/cases/relation/merging_test.rb
+++ b/activerecord/test/cases/relation/merging_test.rb
@@ -118,7 +118,7 @@ class MergingDifferentRelationsTest < ActiveRecord::TestCase
 
   test "merging where relations" do
     hello_by_bob = Post.where(body: "hello").joins(:author).
-      merge(Author.where(name: "Bob")).order("posts.id").pluck("posts.id")
+      merge(Author.where(name: "Bob")).order("posts.id").pluck(Arel.sql("posts.id"))
 
     assert_equal [posts(:misc_by_bob).id,
                   posts(:other_by_bob).id], hello_by_bob
@@ -126,19 +126,19 @@ class MergingDifferentRelationsTest < ActiveRecord::TestCase
 
   test "merging order relations" do
     posts_by_author_name = Post.limit(3).joins(:author).
-      merge(Author.order(:name)).pluck("authors.name")
+      merge(Author.order(:name)).pluck(Arel.sql("authors.name"))
 
     assert_equal ["Bob", "Bob", "David"], posts_by_author_name
 
     posts_by_author_name = Post.limit(3).joins(:author).
-      merge(Author.order("name")).pluck("authors.name")
+      merge(Author.order("name")).pluck(Arel.sql("authors.name"))
 
     assert_equal ["Bob", "Bob", "David"], posts_by_author_name
   end
 
   test "merging order relations (using a hash argument)" do
     posts_by_author_name = Post.limit(4).joins(:author).
-      merge(Author.order(name: :desc)).pluck("authors.name")
+      merge(Author.order(name: :desc)).pluck(Arel.sql("authors.name"))
 
     assert_equal ["Mary", "Mary", "Mary", "David"], posts_by_author_name
   end
diff --git a/activerecord/test/cases/unsafe_raw_sql_test.rb b/activerecord/test/cases/unsafe_raw_sql_test.rb
index d05f0f12d9..89eb02594a 100644
--- a/activerecord/test/cases/unsafe_raw_sql_test.rb
+++ b/activerecord/test/cases/unsafe_raw_sql_test.rb
@@ -13,7 +13,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_order("title").pluck(:id)
+    assert_equal disabled, Post.order(Arel.sql("title")).pluck(:id)
   end
 
   test "order: allows symbol column name" do
@@ -22,7 +22,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_order(:title).pluck(:id)
+    assert_equal disabled, Post.order(Arel.sql("title")).pluck(:id)
   end
 
   test "order: allows downcase symbol direction" do
@@ -31,7 +31,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_order(title: :asc).pluck(:id)
+    assert_equal disabled, Post.order(Arel.sql("title") => Arel.sql("asc")).pluck(:id)
   end
 
   test "order: allows upcase symbol direction" do
@@ -40,7 +40,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_order(title: :ASC).pluck(:id)
+    assert_equal disabled, Post.order(Arel.sql("title") => Arel.sql("ASC")).pluck(:id)
   end
 
   test "order: allows string direction" do
@@ -49,7 +49,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_order(title: "asc").pluck(:id)
+    assert_equal disabled, Post.order(Arel.sql("title") => Arel.sql("asc")).pluck(:id)
   end
 
   test "order: allows multiple columns" do
@@ -58,7 +58,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_order(:author_id, :title).pluck(:id)
+    assert_equal disabled, Post.order(Arel.sql("author_id"), Arel.sql("title")).pluck(:id)
   end
 
   test "order: allows mixed" do
@@ -67,12 +67,12 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_order(:author_id, title: :asc).pluck(:id)
+    assert_equal disabled, Post.order(Arel.sql("author_id"), Arel.sql("title") => Arel.sql("asc")).pluck(:id)
   end
 
   test "order: disallows invalid column name" do
     with_config(:disabled) do
-      assert_raises(ArgumentError) do
+      assert_raises(ActiveRecord::UnknownAttributeReference) do
         Post.order("title asc").pluck(:id)
       end
     end
@@ -88,19 +88,37 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
 
   test "order: disallows invalid column with direction" do
     with_config(:disabled) do
-      assert_raises(ArgumentError) do
+      assert_raises(ActiveRecord::UnknownAttributeReference) do
         Post.order(foo: :asc).pluck(:id)
       end
     end
   end
 
+  test "order: always allows Arel" do
+    enabled, disabled = with_configs(:enabled, :disabled) do
+      Post.order(Arel.sql("length(title)")).pluck(:title)
+    end
+
+    assert_equal enabled, disabled
+  end
+
+  test "order: logs deprecation warning for unrecognized column" do
+    with_config(:deprecated) do
+      ActiveSupport::Deprecation.expects(:warn).with do |msg|
+        msg =~ /\ADangerous query method used with .*length\(title\)/
+      end
+
+      Post.order("length(title)")
+    end
+  end
+
   test "pluck: allows string column name" do
     enabled, disabled = with_configs(:enabled, :disabled) do
       Post.pluck("title")
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_pluck("title")
+    assert_equal disabled, Post.pluck(Arel.sql("title"))
   end
 
   test "pluck: allows symbol column name" do
@@ -109,7 +127,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_pluck(:title)
+    assert_equal disabled, Post.pluck(Arel.sql("title"))
   end
 
   test "pluck: allows multiple column names" do
@@ -118,7 +136,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_pluck(:title, :id)
+    assert_equal disabled, Post.pluck(Arel.sql("title"), Arel.sql("id"))
   end
 
   test "pluck: allows column names with includes" do
@@ -127,7 +145,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.includes(:comments).unsafe_raw_pluck(:title, :id)
+    assert_equal disabled, Post.includes(:comments).pluck(Arel.sql("title"), Arel.sql("id"))
   end
 
   test "pluck: allows auto-generated attributes" do
@@ -136,12 +154,12 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
     end
 
     assert_equal enabled, disabled
-    assert_equal disabled, Post.unsafe_raw_pluck(:tags_count)
+    assert_equal disabled, Post.pluck(Arel.sql("tags_count"))
   end
 
   test "pluck: disallows invalid column name" do
     with_config(:disabled) do
-      assert_raises(ArgumentError) do
+      assert_raises(ActiveRecord::UnknownAttributeReference) do
         Post.pluck("length(title)")
       end
     end
@@ -149,7 +167,7 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
 
   test "pluck: disallows invalid column name amongst valid names" do
     with_config(:disabled) do
-      assert_raises(ArgumentError) do
+      assert_raises(ActiveRecord::UnknownAttributeReference) do
         Post.pluck(:title, "length(title)")
       end
     end
@@ -157,12 +175,30 @@ class UnsafeRawSqlTest < ActiveRecord::TestCase
 
   test "pluck: disallows invalid column names with includes" do
     with_config(:disabled) do
-      assert_raises(ArgumentError) do
+      assert_raises(ActiveRecord::UnknownAttributeReference) do
         Post.includes(:comments).pluck(:title, "length(title)")
       end
     end
   end
 
+  test "pluck: always allows Arel" do
+    enabled, disabled = with_configs(:enabled, :disabled) do
+      Post.includes(:comments).pluck(:title, Arel.sql("length(title)"))
+    end
+
+    assert_equal enabled, disabled
+  end
+
+  test "pluck: logs deprecation warning" do
+    with_config(:deprecated) do
+      ActiveSupport::Deprecation.expects(:warn).with do |msg|
+        msg =~ /\ADangerous query method used with .*length\(title\)/
+      end
+
+      Post.includes(:comments).pluck(:title, "length(title)")
+    end
+  end
+
   def with_configs(*new_values, &blk)
     new_values.map { |nv| with_config(nv, &blk) }
   end