photocrowd / django-cursor-pagination

Cursor-based pagination for Django
BSD 3-Clause "New" or "Revised" License
156 stars 27 forks source link

On mysql, compare CONCAT'd values rather than tuple of values #14

Closed joealcorn closed 6 years ago

joealcorn commented 6 years ago

When ordering by two or more fields as well as supplying a cursor argument to the CurorPaginator.page function, the ORM will generate a query something along the lines of this

SELECT `project`.`id`,
       `project`.`name`,
       `project`.`date_created`,
       (`project`.`date_created`, `project`.`id`) AS `_cursor`
FROM `project`
WHERE ((`project`.`date_created`, `project`.`id`) > (2017-02-16 17:41:42+00:00, 1739438))
ORDER BY `project`.`date_created` DESC,
         `project`.`id` DESC

On mysql this breaks at the AS '_cursor' part - OperationalError: (1241, 'Operand should contain 1 column(s)').

It seems mysql doesn't allow you to select a tuple. To get around this, I'm concatenating the values instead.

With this change, the query generated looks more like this (as_mysql logic was taken from here)

SELECT `project`.`id`,
       `project`.`name`,
       `project`.`date_created`,
       CONCAT_WS('', `project`.`date_created`, `project`.`id`) AS `_cursor`
FROM `project`
WHERE (CONCAT_WS('', `project`.`date_created`, `project`.`id`) < (CONCAT_WS('', 2017-02-16 17:41:42+00:00, 1739438)))
ORDER BY `project`.`date_created` DESC,
         `project`.`id` DESC

From my limited testing, this does seem to work (I'm using django 1.9.13), but I'm not 100% certain of the performance impact or potential comparison differences of using concat rather than comparing tuples.

mjtamlyn commented 6 years ago

That sounds very fragile to me, not all types will sort in the correct way when they are concatenated as strings. (e.g. 20 < 3).

If the tuple approach doesn't work on myself, then the project would need a lot more work to manually reconstruct the cursor. Personally I'd rather document this as a limitation of the project.

joealcorn commented 6 years ago

Fair enough, you are right it is fragile, I've run into an edge case already (and expecting more). I may have to put some work in to get it creating proper where queries on mysql, if I do I'll see about contributing those changes upstream. In the mean time I'll close this PR. :)