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

Relation#last doesn't respect as #64

Closed
jodosha opened this issue May 23, 2016 · 7 comments
Closed

Relation#last doesn't respect as #64

jodosha opened this issue May 23, 2016 · 7 comments

Comments

@jodosha
Copy link
Collaborator

jodosha commented May 23, 2016

I'm using rom-sql via rom-repository as integration with hanami-model.

From my repository, if I use #first with as it works fine:

def first
  users.as(:entity).first
end

###

repository.first.class # => User

But this isn't respected with #last:

def last
  users.as(:entity).last
end

###

repository.last.class # => Hash

To overcome to this problem, I implemented #last as follow:

  def last
    users.order(Sequel.desc(users.primary_key)).as(:entity).first
  end

The complete setup can be found here: https://github.com/hanami/model/blob/6ea119ebc308cd745fa03d5e7729678bc16d6dd2/test/support/fixtures.rb#L28

@solnic
Copy link
Member

solnic commented Jan 27, 2017

Sorry for not addressing this, it's more tricky than it seems to be, because first and last are implemented in relations, and they already return tuples, so they can't go through mapping pipeline.

Notice that first materializes the whole relation, then returns its first element. This means that all tuples are fetched - so, better not to use it.

We have a dedicated API for fetching a single tuple: one and one! and this should be used instead of first and last.

I would suggest doing this if you really want to have these methods available on repos with automapping:

def first
  users.limit(1).as(:entity).first
end

def last
  users.limit(1).as(:entity).reverse.first
end

I'll close this, it's not going to be changed in rom. I may even remove first and last in rom-sql 2.0.0 because it may cause confusion. I'll report a new issue explaining this problem and ref this one.

@solnic solnic closed this as completed Jan 27, 2017
@jodosha
Copy link
Collaborator Author

jodosha commented Jan 27, 2017

@solnic Thanks for getting back. No worries. 👍

@cllns
Copy link

cllns commented Feb 23, 2017

Thanks for explaining this @solnic. It's something I hadn't thought of, but it's a terrific insight.

I have one misunderstanding. It's probably my fault :)

Did you mean to have the example code-block as:

def first
  users.limit(1).as(:entity).one
end

def last
  users.limit(1).as(:entity).reverse.one
end

(instead of calling first, because you said first materializes the whole relation, then returns its first element.)

Or does first act differently after limit and as?

@solnic
Copy link
Member

solnic commented Feb 23, 2017

Or does first act differently after limit and as?

since we add limit then it'll be only one tuple :)

@cllns
Copy link

cllns commented Feb 23, 2017

ah, I see. so it doesn't matter because you're just materializing a single element

@solnic
Copy link
Member

solnic commented Feb 23, 2017

@cllns exactly. I should mention that one has different semantics because it expects that there will be a single tuple, and will raise if there's more than one, and one! will raise when there's no tuple or more than one. So it's a very intentional method (and actually saves you from bugs).

@cllns
Copy link

cllns commented Feb 23, 2017

thanks! Yes, I saw that distinction and understand it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants