Defer Arel attribute lookup to the model class

[matthewd]

This still isn't as separated as I'd like, but it at least moves most of the burden of alias mapping in one place.

@sgrif I was initially expecting has_attribute?(an_alias) to be true -- which would save us the two remaining places we explicitly ask whether we have an alias.

.. and then there's the one that needs a list of all "arel resolvable" attribute names (i.e., including aliases).

[sgrif]

I'd still really prefer that we had TableMetadata be the point of separation for this -- I don't like the model class itself being the point of coupling for AST builder related functions. That said, if we really want to go this route, we could probably give the second argument a default at least.

[matthewd]

I'm going via the TableMetadata where that's already how we were getting the columns. Here I'm just aiming for a simple extract (and unify) method refactoring, without rearranging anything overall.

As it currently stands, the TableMetadata already relies on the klass for aliases.

Overall, I think this is not much of an improvement, but not an architectural regression.

[sgrif]

My concern with it is that it creates the klass as the sole place to override this behavior, not TableMetadata.

[sgrif]

Anyway I don't feel super strongly about this. Go with whatever you think is best.

[matthewd]

My thinking is that it's all private anyway: once we've taught the other places to use the tablemetadata, then we stop it delegating to klass, and kill the method on the model; this just works as a small interim step.

[sgrif]

👍

[kbrock]

thanks @sgrif and @matthewd