Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActiveRecord::Calculations#pluck accepts multiple columns #6500

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 19 additions & 9 deletions activerecord/lib/active_record/relation/calculations.rb
Expand Up @@ -127,6 +127,12 @@ def calculate(operation, column_name, options = {})
# # SELECT people.id FROM people
# # => [1, 2, 3]
#
# Person.pluck([:id, :name])
# # SELECT people.id, people.name FROM people
#
# Person.pluck(:id, :name)
# # SELECT people.id, people.name FROM people
#
# Person.uniq.pluck(:role)
# # SELECT DISTINCT role FROM people
# # => ['admin', 'member', 'guest']
Expand All @@ -139,11 +145,13 @@ def calculate(operation, column_name, options = {})
# # SELECT DATEDIFF(updated_at, created_at) FROM people
# # => ['0', '27761', '173']
#
def pluck(column_name)
if column_name.is_a?(Symbol) && column_names.include?(column_name.to_s)
column_name = "#{table_name}.#{column_name}"
def pluck(*column_name)
if column_name.size == 1 && column_name.first.is_a?(Symbol) && column_names.include?(column_name.first.to_s)
column_name = "#{table_name}.#{column_name.first}"
elsif column_name.is_a?(Array)
column_name.flatten!
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same check to add the table name has to happen for each column name, otherwise the method would work differently for each case.


result = klass.connection.select_all(select(column_name).arel, nil, bind_values)

key = result.columns.first
Expand All @@ -154,11 +162,13 @@ def pluck(column_name)
}

result.map do |attributes|
raise ArgumentError, "Pluck expects to select just one attribute: #{attributes.inspect}" unless attributes.one?

value = klass.initialize_attributes(attributes).values.first

column.type_cast(value)
if attributes.one?
value = klass.initialize_attributes(attributes).values.first

column.type_cast(value)
else
column_name.collect{|c| attributes[c.to_s]}
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Columns need to be typecasted in both scenarios, otherwise pluck would work differently when using 1 and more columns.

end
end

Expand Down
19 changes: 17 additions & 2 deletions activerecord/test/cases/calculations_test.rb
Expand Up @@ -477,11 +477,26 @@ def test_pluck_with_selection_clause
assert_equal [50 + 53 + 55 + 60], Account.pluck('SUM(DISTINCT(credit_limit)) as credit_limit')
end

def test_pluck_expects_a_single_selection
assert_raise(ArgumentError) { Account.pluck 'id, credit_limit' }
def test_pluck_expects_a_single_or_multiple_selection
assert_nothing_raised(ArgumentError) { Account.pluck 'id, credit_limit' }
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to assert for nothing raised, just make sure you're getting the right response from the sql piece. Try to test some different sqls as well.


def test_plucks_with_ids
assert_equal Company.all.map(&:id).sort, Company.ids.sort
end

def test_pluck_multiple_columns
assert_equal [
[1, "The First Topic"], [2, "The Second Topic of the day"],
[3, "The Third Topic of the day"], [4, "The Fourth Topic of the day"]
], Topic.order(:id).pluck([:id, :title])
assert_equal [
[1, "The First Topic", "David"], [2, "The Second Topic of the day", "Mary"],
[3, "The Third Topic of the day", "Carl"], [4, "The Fourth Topic of the day", "Carl"]
], Topic.order(:id).pluck([:id, :title, :author_name])
assert_equal [
[1, "The First Topic"], [2, "The Second Topic of the day"],
[3, "The Third Topic of the day"], [4, "The Fourth Topic of the day"]
], Topic.order(:id).pluck(:id, :title)
end
end