Skip to content

Commit

Permalink
Remove ORDER BY clause from COUNT queries
Browse files Browse the repository at this point in the history
Since we just used the pre-assembled relations to then call `#size` on
them, this caused queries like this to be sent to the database:
```
SELECT COUNT(*) FROM (
  SELECT 1 AS one
  FROM `posts`
  ORDER BY `posts`.`author` DESC, `posts`.`id` DESC
  LIMIT 3
) subquery_for_count
```

The `ORDER BY` part will never influence the number of returned records.
But depending on the used database and their query optimization
settings, it will make the query less performant.

Therefore, remove any order from the queries before calling `#size` on
them. With `ActiveRecord` this can be achieved by calling `#reorder` on
the relation with an empty string.
  • Loading branch information
nicolas-fricke committed Apr 19, 2021
1 parent 4009f7e commit dfc7e96
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ These are the latest changes on the project's `master` branch that have not yet

### Changed
- **Breaking change:** The way records are retrieved from a given cursor has been changed to no longer use `CONCAT` but instead simply use a compound `WHERE` clause in case of a custom order and having both the custom field as well as the `id` field in the `ORDER BY` query. This is a breaking change since it now changes the internal order of how records with the same value of the `order_by` field are returned.
- Remove `ORDER BY` clause from `COUNT` queries

## [0.1.3] - 2021-03-17

Expand Down
11 changes: 6 additions & 5 deletions lib/rails_cursor_pagination/paginator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def page
#
# @return [Integer]
def total
memoize(:total) { @relation.size }
memoize(:total) { @relation.reorder('').size }
end

# Check if the pagination direction is forward
Expand All @@ -186,11 +186,12 @@ def previous_page?
# When paginating forward, we can only have a previous page if we were
# provided with a cursor and there were records discarded after applying
# this filter. These records would have to be on previous pages.
@cursor.present? && filtered_and_sorted_relation.size < total
@cursor.present? &&
filtered_and_sorted_relation.reorder('').size < total
else
# When paginating backwards, if we managed to load one more record than
# requested, this record will be available on the previous page.
@page_size < limited_relation_plus_one.size
@page_size < limited_relation_plus_one.reorder('').size
end
end

Expand All @@ -201,12 +202,12 @@ def next_page?
if paginate_forward?
# When paginating forward, if we managed to load one more record than
# requested, this record will be available on the next page.
@page_size < limited_relation_plus_one.size
@page_size < limited_relation_plus_one.reorder('').size
else
# When paginating backward, if applying our cursor reduced the number
# records returned, we know that the missing records will be on
# subsequent pages.
filtered_and_sorted_relation.size < total
filtered_and_sorted_relation.reorder('').size < total
end
end

Expand Down

0 comments on commit dfc7e96

Please sign in to comment.