Use attribute_names over column_names

[kbrock]

Summary

Rails know how to convert a column name to an Arel::Attribute.
The rest of the code base uses arel_attribute over Arel::Attribute.new.
This patch cleans up this one spot.

//cc @matthewd was able to remove the aliases code
/cc @sgrif Do you feel this is too private api focused? Any suggestions for a different approach? thnx

[rails-bot]

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @sgrif (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[sgrif]

Yeah I think this is too private API focused. Overall I'm trying to reduce the amount of places where "an active record class" is the interface desired. column_names is the right thing to be using here, since virtual attributes aren't in the database.

[kbrock]

Overall I'm trying to reduce the amount of places where "an active record class" is the interface desired.

Did you just say that you prefer using column_names over attribute_names?
Or were you talking about avoiding arel_attribute specifically?

virtual attributes aren't in the database.

arel_attribute does it's best to represent an attribute in terms of the database. E.g.: It converts an alias to the proper column name.

So while yes, you don't want to pass a sql expression to arel_attribute, it would be nice to leverage the method's logic for converting things like an attribute alias to a column name. Which will work in the database.

[kbrock]

@sgrif Hi again.

I thought pluck [ref] is implemented nicely:

relation.select_values = column_names.map { |cn|
  @klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn
}

Used the same logic in aggregate_column

if @klass.has_attribute?(column_name.to_s) || @klass.attribute_alias?(column_name.to_s)
  @klass.arel_attribute(column_name)
else
  Arel.sql(column_name == :all ? "*" : column_name.to_s)
end

• this makes the logic for the two methods consistent
• Was able to remove the alias conversion. (I can put back if you want)