Merge pull request #13886 from arthurnn/fix_relation_arel

Fix regression on .select method

8 files changed, 52 insertions, 18 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 7df4720ea5..a6400a169b 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,11 @@
+*   Fix regressions on `select_*` methods.
+    When `select_*` methods receive a `Relation` object, they should be able to get the arel/binds from it.
+    Also fix regressions on select_rows that was ignoring the binds.
+
+    Fixes #7538, #12017, #13731, #12056.
+
+    *arthurnn*
+
 *   Active Record objects can now be correctly dumped, loaded and dumped again without issues.
 
     Previously, if you did `YAML.dump`, `YAML.load` and then `YAML.dump` again
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
index c90915c509..6eb59cc398 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -20,6 +20,14 @@ module ActiveRecord
 
       # Returns an ActiveRecord::Result instance.
       def select_all(arel, name = nil, binds = [])
+        if arel.is_a?(Relation)
+          relation = arel
+          arel = relation.arel
+          if !binds || binds.empty?
+            binds = relation.bind_values
+          end
+        end
+
         select(to_sql(arel, binds), name, binds)
       end
 
@@ -39,13 +47,16 @@ module ActiveRecord
       # Returns an array of the values of the first column in a select:
       #   select_values("SELECT id FROM companies LIMIT 3") => [1,2,3]
       def select_values(arel, name = nil)
-        select_rows(to_sql(arel, []), name)
-          .map { |v| v[0] }
+        binds = []
+        if arel.is_a?(Relation)
+          arel, binds = arel.arel, arel.bind_values
+        end
+        select_rows(to_sql(arel, binds), name, binds).map(&:first)
       end
 
       # Returns an array of arrays containing the field values.
       # Order is the same as that returned by +columns+.
-      def select_rows(sql, name = nil)
+      def select_rows(sql, name = nil, binds = [])
       end
       undef_method :select_rows
 
diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
index 6d8e994654..b07b0cb826 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
@@ -213,7 +213,7 @@ module ActiveRecord
 
       # Returns an array of arrays containing the field values.
       # Order is the same as that returned by +columns+.
-      def select_rows(sql, name = nil)
+      def select_rows(sql, name = nil, binds = [])
         execute(sql, name).to_a
       end
 
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 7dbaa272a3..49f0bfbcde 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -213,9 +213,9 @@ module ActiveRecord
 
       # DATABASE STATEMENTS ======================================
 
-      def select_rows(sql, name = nil)
+      def select_rows(sql, name = nil, binds = [])
         @connection.query_with_result = true
-        rows = exec_query(sql, name).rows
+        rows = exec_query(sql, name, binds).rows
         @connection.more_results && @connection.next_result    # invoking stored procedures with CLIENT_MULTI_RESULTS requires this to tidy up else connection will be dropped
         rows
       end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb
index f349c37724..51ee2829b2 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb
@@ -46,8 +46,8 @@ module ActiveRecord
 
         # Executes a SELECT query and returns an array of rows. Each row is an
         # array of field values.
-        def select_rows(sql, name = nil)
-          select_raw(sql, name).last
+        def select_rows(sql, name = nil, binds = [])
+          exec_query(sql, name, binds).rows
         end
 
         # Executes an INSERT query and returns the new record's ID
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index a471383041..9618ba4087 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -942,14 +942,6 @@ module ActiveRecord
           exec_query(sql, name, binds)
         end
 
-        def select_raw(sql, name = nil)
-          res = execute(sql, name)
-          results = result_as_array(res)
-          fields = res.fields
-          res.clear
-          return fields, results
-        end
-
         # Returns the list of a table's column names, data types, and default values.
         #
         # The underlying query is roughly:
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index 170dddb08e..3c5f7a981e 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -347,8 +347,8 @@ module ActiveRecord
       end
       alias :create :insert_sql
 
-      def select_rows(sql, name = nil)
-        exec_query(sql, name).rows
+      def select_rows(sql, name = nil, binds = [])
+        exec_query(sql, name, binds).rows
       end
 
       def begin_db_transaction #:nodoc:
diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb
index b67e70ec7e..0eb1231c79 100644
--- a/activerecord/test/cases/adapter_test.rb
+++ b/activerecord/test/cases/adapter_test.rb
@@ -1,5 +1,7 @@
 require "cases/helper"
 require "models/book"
+require "models/post"
+require "models/author"
 
 module ActiveRecord
   class AdapterTest < ActiveRecord::TestCase
@@ -179,6 +181,27 @@ module ActiveRecord
       assert result.is_a?(ActiveRecord::Result)
     end
 
+    def test_select_methods_passing_a_association_relation
+      author = Author.create!(name: 'john')
+      Post.create!(author: author, title: 'foo', body: 'bar')
+      query = author.posts.select(:title)
+      assert_equal({"title" => "foo"}, @connection.select_one(query.arel, nil, query.bind_values))
+      assert_equal({"title" => "foo"}, @connection.select_one(query))
+      assert @connection.select_all(query).is_a?(ActiveRecord::Result)
+      assert_equal "foo", @connection.select_value(query)
+      assert_equal ["foo"], @connection.select_values(query)
+    end
+
+    def test_select_methods_passing_a_relation
+      Post.create!(title: 'foo', body: 'bar')
+      query = Post.where(title: 'foo').select(:title)
+      assert_equal({"title" => "foo"}, @connection.select_one(query.arel, nil, query.bind_values))
+      assert_equal({"title" => "foo"}, @connection.select_one(query))
+      assert @connection.select_all(query).is_a?(ActiveRecord::Result)
+      assert_equal "foo", @connection.select_value(query)
+      assert_equal ["foo"], @connection.select_values(query)
+    end
+
     test "type_to_sql returns a String for unmapped types" do
       assert_equal "special_db_type", @connection.type_to_sql(:special_db_type)
     end