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

Fetch attribute by alias #524

Open
waiting-for-dev opened this issue Apr 18, 2019 · 5 comments
Open

Fetch attribute by alias #524

waiting-for-dev opened this issue Apr 18, 2019 · 5 comments

Comments

@waiting-for-dev
Copy link
Collaborator

waiting-for-dev commented Apr 18, 2019

As you know from previous issues and PR's, I'm trying to make rom a complete isolation layer from the column names used in the persistence layer (besides the configuration of the scheme, of course). For this reason, I would like to be able to fetch in the most transparent possible way an attribute by its aliased name.

Examples

Right now, if you have:

attribute :foo, alias: :bar

you can do relation[:foo] but not relation[:bar].

This is very annoying when building queries, because you have to remember to use the persistence layer name each time you use an aliased attribute.

Ideally, I would like to add steroids to the #[] method so that it does a second lookup by alias if nothing is found with given name. However, I still like the idea of having a more pure method which only finds by attribute name, which I think should be used mainly internally in rom implementation.

We could do two things here:

  • Change #[] to lookup by name and alias and use another #fetch_by_name internally.
  • Keep #[] as it is and add another method #fetch_with_alias or just #>> which would lookup by name and alias. However, from the user point of view, recommending using both #[] and #fetch_with_alias would not be transparent and would make the code harder to read (because #[] is very convenient).
@solnic
Copy link
Member

solnic commented Apr 23, 2019

This is very annoying when building queries, because you have to remember to use the persistence layer name each time you use an aliased attribute.

This can be hidden by your relation API. That's the whole point of having this layer - hide details. I'm not sure if there's anything we should do about it to be honest, but maybe I'm wrong.

However, I still like the idea of having a more pure method which only finds by attribute name, which I think should be used mainly internally in rom implementation

Why do you think that? It's very important to have a method that returns attributes without any ambiguity. Adding the second lookup to #[] is not a good idea because of the ambiguity it would introduce.

Take this as an example why it's important for #[] to be 100% obvious:

[7] pry(main)> users.join(tasks).select(*users.schema, *tasks.schema.rename(title: :name)).schema.map(&:name)
=> [:id, :name, :id, :user_id, :title]

[8] pry(main)> users.join(tasks).select(*users.schema, *tasks.schema.rename(title: :name)).schema[:title]
=> #<ROM::SQL::Attribute[NilClass | String] name=:title source=ROM::Relation::Name(tasks) qualified=true sql_expr=#<Sequel::SQL::AliasedExpression @expression=>#<Sequel::SQL::QualifiedIdentifier @table=>:tasks, @column=>:title>, @alias=>:name, @columns=>nil> alias=:name>

[9] pry(main)> users.join(tasks).select(*users.schema, *tasks.schema.rename(title: :name)).schema[:name]
=> #<ROM::SQL::Attribute[NilClass | String] name=:name source=ROM::Relation::Name(users) qualified=true sql_expr=#<Sequel::SQL::QualifiedIdentifier @table=>:users, @column=>:name> alias=nil>

Now imagine if #[] would perform a second lookup, what would it return? First-match? All matches? Should it raise an error when there's more than 1 match?

@waiting-for-dev
Copy link
Collaborator Author

waiting-for-dev commented Apr 24, 2019

Thanks for taking this into consideration.

This can be hidden by your relation API. That's the whole point of having this layer - hide details. I'm not sure if there's anything we should do about it to be honest, but maybe I'm wrong.

It is fair. The difference with my view is that I'd like to be more granular and hide it in the schema sub-layer within the relation layer. My interest with it comes because I have been working with a legacy database with extremely ugly names, mixing even different languages in the column names. For this reason, I'd like to finish with the pain in my eyes in one single line:

attribute :i_m_super_ugly, alias: :i_m_so_nice

From this moment on, I'd like to be possible not to use i_m_super_ugly anymore, but with the current behavior of #[] (and I think also of dynamic DSL methods which I haven't used, maybe food for another issue) I see it cluttering my relation methods.

Then there is also some cross-layer coupling I have been using because sometimes it is too convenient to avoid. For example, say you are rendering an HTML table containing data from a relation and this table have links to be ordered by any of the columns, where each column represents a relation attribute. Currently I'm using the attribute name to build the link, and it is travelling unmodified from the view layer bottom to the relation layer, where it finishes its voyage within #[]. This coupling has parameter sanitizing in the middle but it hasn't any parameter-attribute mapping. Here, as exception to the general rule, I do prefer to rely on convention over having to define mappings at each layer boundary. With the current behaviour, I have to see :i_m_super_ugly even in my views.

Now imagine if #[] would perform a second lookup, what would it return? First-match? All matches? Should it raise an error when there's more than 1 match?

I think it is similar to what currently is happening with this slightly modified example:

[1] pry(main)> users.join(tasks).select(*users.schema, *tasks.schema).schema.map(&:name)
=> [:id, :name, :id, :user_id, :name]
[2] pry(main)> users.join(tasks).select(*users.schema, *tasks.schema).schema[:name]

Right now it is returning first-match.

With my original idea, #[] would not do the second lookup when something is found by name. For this reason I thought that something with less powers should be kept, like a #fetch_by_name method. But we can think more about the ideal implementation. Right now, Relation#[] is just delegating to Schema#[]. Probably, if we implement the proposed feature, it would make sense to keep Schema#[] as it is and just change Relation#[] to take into consideration aliased names and even forget about the original names. Of course, this information should be provided by the schema with some kind of mapping between final names and attributes.

@solnic
Copy link
Member

solnic commented Apr 27, 2019

I think it is similar to what currently is happening with this slightly modified example

I reported #529 about this

Probably, if we implement the proposed feature, it would make sense to keep Schema#[] as it is and just change Relation#[] to take into consideration aliased names and even forget about the original names

Actually, this may work. It's worth experimenting with this approach.

@dsisnero
Copy link

dsisnero commented Sep 3, 2020

I need this too because I have to use it with lots of legacy databases. I would like to use the aliased attribute in all my database interactions like was done in the datamapper days.

in my associations, in my joins, in everything

@solnic
Copy link
Member

solnic commented Sep 4, 2020

@dsisnero we'll get to this eventually!

@solnic solnic modified the milestones: 6.0.0, 7.0.0 Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants