Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix #5990 Against 3-2 Stable #6088

Closed
wants to merge 1 commit into from

4 participants

@ahawkins

Title says it all. Same thing as #6086, but working against 3-2 stable. This patch also appends the table name if a symbol is given, so pluck(:id) will select the id from the correct table when includes/joins are used.

@ahawkins

/cc @jonleighton @tenderlove any love for a backport fix for 3.2.4?

@carlosantoniodasilva carlosantoniodasilva commented on the diff
activerecord/lib/active_record/relation/calculations.rb
@@ -177,9 +177,17 @@ def calculate(operation, column_name, options = {})
# Person.where(:confirmed => true).limit(5).pluck(:id)
#
def pluck(column_name)
- column_name = column_name.to_s
- klass.connection.select_all(select(column_name).arel).map! do |attributes|
- klass.type_cast_attribute(attributes.keys.first, klass.initialize_attributes(attributes))
+
+ if column_name.is_a?(Symbol) && column_names.include?(column_name.to_s)
+ column_name = "#{table_name}.#{column_name}"
+ end
+
+ if eager_loading? || (includes_values.present? && references_eager_loaded_tables?)
+ return construct_relation_for_association_calculations.pluck(column_name)

No need for return here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rafaelfranca

I don't think that is safer to backport a7e715e to 3-2-stable. I'm closing this.

Thanks @twinturbo

@parndt

Doesn't appear like this was this ever merged to rails/rails?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 30, 2012
  1. Close #5990 Against 3.2 stable

    twinturbo authored
This page is out of date. Refresh to see the latest.
View
14 activerecord/lib/active_record/relation/calculations.rb
@@ -177,9 +177,17 @@ def calculate(operation, column_name, options = {})
# Person.where(:confirmed => true).limit(5).pluck(:id)
#
def pluck(column_name)
- column_name = column_name.to_s
- klass.connection.select_all(select(column_name).arel).map! do |attributes|
- klass.type_cast_attribute(attributes.keys.first, klass.initialize_attributes(attributes))
+
+ if column_name.is_a?(Symbol) && column_names.include?(column_name.to_s)
+ column_name = "#{table_name}.#{column_name}"
+ end
+
+ if eager_loading? || (includes_values.present? && references_eager_loaded_tables?)
+ return construct_relation_for_association_calculations.pluck(column_name)

No need for return here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ else
+ klass.connection.select_all(select(column_name).arel).map! do |attributes|
+ klass.type_cast_attribute(attributes.keys.first, klass.initialize_attributes(attributes))
+ end
end
end
View
5 activerecord/test/cases/calculations_test.rb
@@ -478,4 +478,9 @@ def test_pluck_with_serialization
def test_pluck_with_qualified_column_name
assert_equal [1,2,3,4], Topic.order(:id).pluck("topics.id")
end
+
+ def test_pluck_if_table_included
+ c = Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)])
+ assert_equal [c.id], Company.includes(:contracts).where("contracts.id" => c.contracts.first).pluck(:id)
+ end
end
Something went wrong with that request. Please try again.