Merge pull request #25522 from kamipo/fix_select_rows_method_signature_for_consistency

Fix `select_rows` method signature for consistency

3 files changed, 24 insertions, 20 deletions

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 2c352819fb..769f488469 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -51,8 +51,7 @@ module ActiveRecord
 
       # Returns a single value from a record
       def select_value(arel, name = nil, binds = [])
-        arel, binds = binds_from_relation arel, binds
-        if result = select_rows(to_sql(arel, binds), name, binds).first
+        if result = select_rows(arel, name, binds).first
           result.first
         end
       end
@@ -60,14 +59,13 @@ 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, binds = [])
-        arel, binds = binds_from_relation arel, binds
-        select_rows(to_sql(arel, binds), name, binds).map(&:first)
+        select_rows(arel, 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, binds = [])
-        exec_query(sql, name, binds).rows
+      def select_rows(arel, name = nil, binds = [])
+        select_all(arel, name, binds).rows
       end
 
       # Executes the SQL statement in the context of this connection and returns
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb
index 78e7181266..8c67a7a80b 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/database_statements.rb
@@ -3,7 +3,7 @@ module ActiveRecord
     module MySQL
       module DatabaseStatements
         # Returns an ActiveRecord::Result instance.
-        def select_all(arel, name = nil, binds = [], preparable: nil)
+        def select_all(arel, name = nil, binds = [], preparable: nil) # :nodoc:
           result = if ExplainRegistry.collect? && prepared_statements
             unprepared_statement { super }
           else
@@ -15,8 +15,8 @@ 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, binds = [])
-          select_result(sql, name, binds) do |result|
+        def select_rows(arel, name = nil, binds = []) # :nodoc:
+          select_result(arel, name, binds) do |result|
             @connection.next_result while @connection.more_results?
             result.to_a
           end
@@ -58,7 +58,9 @@ module ActiveRecord
             @connection.last_id
           end
 
-          def select_result(sql, name = nil, binds = [])
+          def select_result(arel, name, binds)
+            arel, binds = binds_from_relation(arel, binds)
+            sql = to_sql(arel, binds)
             if without_prepared_statement?(binds)
               execute_and_free(sql, name) { |result| yield result }
             else
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 9a2017b443..705e6063dc 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb
@@ -7,18 +7,14 @@ module ActiveRecord
           PostgreSQL::ExplainPrettyPrinter.new.pp(exec_query(sql, "EXPLAIN", binds))
         end
 
-        def select_value(arel, name = nil, binds = [])
-          arel, binds = binds_from_relation arel, binds
-          sql = to_sql(arel, binds)
-          execute_and_clear(sql, name, binds) do |result|
+        def select_value(arel, name = nil, binds = []) # :nodoc:
+          select_result(arel, name, binds) do |result|
             result.getvalue(0, 0) if result.ntuples > 0 && result.nfields > 0
           end
         end
 
-        def select_values(arel, name = nil, binds = [])
-          arel, binds = binds_from_relation arel, binds
-          sql = to_sql(arel, binds)
-          execute_and_clear(sql, name, binds) do |result|
+        def select_values(arel, name = nil, binds = []) # :nodoc:
+          select_result(arel, name, binds) do |result|
             if result.nfields > 0
               result.column_values(0)
             else
@@ -29,8 +25,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, binds = [])
-          execute_and_clear(sql, name, binds) do |result|
+        def select_rows(arel, name = nil, binds = []) # :nodoc:
+          select_result(arel, name, binds) do |result|
             result.values
           end
         end
@@ -179,6 +175,14 @@ module ActiveRecord
           def suppress_composite_primary_key(pk)
             pk unless pk.is_a?(Array)
           end
+
+          def select_result(arel, name, binds)
+            arel, binds = binds_from_relation(arel, binds)
+            sql = to_sql(arel, binds)
+            execute_and_clear(sql, name, binds) do |result|
+              yield result
+            end
+          end
       end
     end
   end