.last returns wrong result

16 views
Skip to first unread message

lobati

unread,
Jan 13, 2011, 10:33:05 PM1/13/11
to DataMapper
So I'm trying to get the nth result from the database:

a = Task.all(:limit => 10, :order => [ :ordering.asc ]).last

but this query, instead of returning the 10th item actually returns
the last item in the database. I get the same results if I do this:

a = Task.all(:limit => 10, :order => [ :ordering.asc ])
a = a.last

However, I get the results I want if I do something like this:


a = Task.all(:limit => 10, :order => [ :ordering.asc ])
a.each do |task|
# do nothing
end
a = a.last

I gather this is the result of some query optimization going on, but
this is a bug, right? So for the time being, is there are better way
of forcing the query to take place before '.last' and/or is there a
more efficient way for me to get the 10th record with the above
condition?

Robert Fletcher

unread,
Jan 13, 2011, 10:45:40 PM1/13/11
to DataMapper
Well, I discovered this workaround:

a = Task.all(:order => [ :ordering.asc ])[9]

Looks like this optimizes to what the previous one probably ought to.  It uses OFFSET.


--
You received this message because you are subscribed to the Google Groups "DataMapper" group.
To post to this group, send email to datam...@googlegroups.com.
To unsubscribe from this group, send email to datamapper+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/datamapper?hl=en.


RipTheJacker

unread,
Jan 14, 2011, 5:36:21 PM1/14/11
to DataMapper
I don't really think it's a bug. Calling Thing.all.last does a query
for the last item in the db, because Thing#all returns a DM Query, and
the #last updates that query.

Here are ways using mysql: http://www.mysqltutorial.org/select-nth-highest-record-database-table-using-mysql.aspx

As you can see, they all rely on sub queries, some of which I don't
think can be done in DM without writing sql.

Here are other approaches:
Thing.all(:limit =>10).to_a.last <- will find 10 records and return
the last of the array
Thing.all(:limit => 10) - Thing.all(:limit => 9) <- executes a single
query returning an array of the 1 item.

I really think the offset approach you already figured out is the best
though.
> > datamapper+...@googlegroups.com<datamapper%2Bunsubscribe@googlegrou ps.com>
> > .

Robert Fletcher

unread,
Jan 15, 2011, 1:13:21 PM1/15/11
to datam...@googlegroups.com
Hmm, to me 'last' logically operates on the return result of the query, which I would expect to be a collection.  If you were to optimize this:


Task.all(:limit => 10, :order => [ :ordering.asc ]).last

It seems like behind the scenes it should be turned into something like one of these:


Task.all(:order => [ :ordering.asc ])[9]
Task.first(:offset => 9, :order => [ :ordering.asc ])

Both of which optimize to:

SELECT "id", "title", "ordering" FROM "tasks" ORDER BY "ordering" LIMIT 10 OFFSET 9

I'll probably go with the '.first' one since it's clear where the query ends, but just sayin'.  I think it can be done without sub queries.  I don't really think the user should run into DM Query objects unless they're looking for them.

Thanks for a great tool!

To unsubscribe from this group, send email to datamapper+...@googlegroups.com.

RipTheJacker

unread,
Jan 15, 2011, 6:43:17 PM1/15/11
to DataMapper
Yea you're probably right. I made a ticket and will take a look at it:
http://datamapper.lighthouseapp.com/projects/20609-datamapper/tickets/1476-last-ignoring-limit-condition

On Jan 15, 12:13 pm, Robert Fletcher <lobatifri...@gmail.com> wrote:
> Hmm, to me 'last' logically operates on the return result of the query,
> which I would expect to be a collection.  If you were to optimize this:
>
> Task.all(:limit => 10, :order => [ :ordering.asc ]).last
>
> It seems like behind the scenes it should be turned into something like one
> of these:
>
> Task.all(:order => [ :ordering.asc ])[9]
> Task.first(:offset => 9, :order => [ :ordering.asc ])
>
> Both of which optimize to:
>
> SELECT "id", "title", "ordering" FROM "tasks" ORDER BY "ordering" LIMIT 10
> OFFSET 9
>
> I'll probably go with the '.first' one since it's clear where the query
> ends, but just sayin'.  I think it can be done without sub queries.  I don't
> really think the user should run into DM Query objects unless they're
> looking for them.
>
> Thanks for a great tool!
>
>
>
> On Fri, Jan 14, 2011 at 2:36 PM, RipTheJacker <kab...@gmail.com> wrote:
> > I don't really think it's a bug. Calling Thing.all.last does a query
> > for the last item in the db, because Thing#all returns a DM Query, and
> > the #last updates that query.
>
> > Here are ways using mysql:
> >http://www.mysqltutorial.org/select-nth-highest-record-database-table...

Dan Kubb (dkubb)

unread,
Jan 17, 2011, 2:12:04 AM1/17/11
to DataMapper
This does sound like a bug to me.

When I was originally designing the Collection API a couple of years
ago, it was really hard to reason about how things should work until I
started to imagine what would happen in Ruby if I had loaded up the
entire set of resources into an Array and was filtering through that
using Array/Enumerable methods.

So for example, it's a bug if these two statements do not return the
same records:

Task.all(:limit => 10).last
Task.to_a[0, 10].last

One confusing point, that I didn't realize until after everything was
done and we started using the API, was that the Hash based interface
doesn't make it completely clear the operator precedence, eg.

{ :limit => 10, :subscriber => true }

Some people might think this means "limit the results to 10, then
filter subscriber records" OR "filter subscriber records, then limit
those results to 10". I think most people identify with the second
approach, which is similar to what we've had ingrained from SQL usage
(in a SELECT the WHERE clause applies first, then the LIMIT). However,
the first isn't technically wrong.. it's the API which is ambiguous. I
didn't see this ambiguity because I've always identified with the SQL
approach and didn't question it when designing the API.

(I'm looking at ways to reduce this ambiguity in future API revisions,
but I don't have anything concrete yet to demo.)

The following statement should order the tasks by "ordering", then
limit the results to 10, then return the last task from that list:

Task.all(:limit => 10, :order => [ :ordering.asc ]).last

The Hash based interface is a bit unclear, and it could be causing the
bug in how Collection#last should behave. The Collection#first and
Collection#last methods should always evaluate the same as if they
were being called on a collection that was materialized into an Array,
as in Array#first and Array#last.

In fact this might be OT, but not alot of people know this;
Collection#first and Collection#last, like their Array counterparts,
also accept an integer which will allow the first/last n resources to
be returned, eg:

players = Player.all(:order => [ :points ])
players.first(5) # top 5 players
players.last(5) # bottom 5 players

Anyway, to summarize: When in doubt, look at how Array/Enumerable
methods behave and that is how Collection should behave, if possible.

--

Dan
Reply all
Reply to author
Forward
0 new messages