Merge pull request #13886 from arthurnn/fix_relation_arel

Fix regression on .select method

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

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

@@ -20,6 +20,14 @@ def to_sql(arel, binds = [])
 
       # 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 @@ def select_value(arel, name = nil, binds = [])
       # 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
 

activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb

@@ -213,7 +213,7 @@ def build_footer(nrows, elapsed)
 
       # 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
 

activerecord/lib/active_record/connection_adapters/mysql_adapter.rb

@@ -213,9 +213,9 @@ def reset!
 
       # 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

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

@@ -46,8 +46,8 @@ def pp(result)
 
         # 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

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -942,14 +942,6 @@ def select(sql, name = nil, binds = [])
           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:

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -347,8 +347,8 @@ def insert_sql(sql, name = nil, pk = nil, id_value = nil, sequence_name = nil) #
       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:

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 @@ def test_select_all_always_return_activerecord_result
       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