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

Queries with limits and no offset return the last items instead of the first ones #286

Closed
gc-olivierho opened this issue Oct 9, 2013 · 2 comments

Comments

@gc-olivierho
Copy link

When using Redmine with MS SQL, the search results are the 10 last issues instead of the 10 first.
After tracing the Redmine and then Ruby code, I ended up suspecting the following lines, in activerecord-sqlserver-adapter-3.2.12/lib/arel/visitors/sqlserver.rb:

      def visit_Arel_Nodes_SelectStatementWithOutOffset(o, windowed=false)
(...)
        elsif eager_limiting_select_statement?(o)
          projections = projections.map { |x| projection_without_expression(x) }
          groups = projections.map { |x| projection_without_expression(x) }
          orders = orders.map do |x|
            expr = Arel.sql projection_without_expression(x.expr)
======>     x.descending? ? Arel::Nodes::Max.new([expr]) : Arel::Nodes::Min.new([expr])
          end

It seems that if x.descending is TRUE, then we apply a MAX() function, but we should also apply DESC ordering. However, this does not come into the query with this code.

Could someone give his opinion on this?

@gc-olivierho
Copy link
Author

If anyone is still working on that code, or if any user has the same issue, here is how we fixed it on our side:
In activerecord-sqlserver-adapter-3.2.12/lib/arel/visitors/sqlserver.rb, we changed the line 151:

            x.descending? ? Arel::Nodes::Max.new([expr]) : Arel::Nodes::Min.new([expr])

to

            if x.descending?
              Arel::Nodes::Max.new([expr])
              groups += [expr]
              Arel::Nodes::Descending.new([expr])
            else
              Arel::Nodes::Min.new([expr])
            end

This worked for more than a month on our Redmine 2.3.3 site, with 30+ active users.

Please note that I am not a Ruby programmer and have very little knowledge in it. In particular, I still do not understand that code completely and reached that fix after several (if not many) trial and errors. Please anyone feel free to suggest a better way to address the issue I raised.

@metaskills
Copy link
Contributor

There has been major changes in the 4.2 adapter WRT limit/offset since we now use OFFSET/FETCH. This issue is likely resolved in that version. Cheers and more info here. http://metaskills.net/2015/01/25/activerecord-sqlserver-v4.2.0---code-name-kantishna/

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

2 participants