photocrowd / django-cursor-pagination

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

Pagination with null values #44

Closed untidy-hair closed 1 year ago

untidy-hair commented 1 year ago

πŸ‘‹ First of all, thank you for this awesome library!

I encountered this None/Null issue at my workplace (modified) where the last object of the previous page has NULL value for one of the ordering keys.

ERROR:  invalid input syntax for type smallint: "None" at character 1564

... AND ("product"."tier" > 'None' OR ("product"."tier" = 'None' AND "product"."id" > '555'))) ORDER BY "product"."tier" ASC, "product"."id" DESC LIMIT 101

I made a patch and that is working for me but now I am trying to update this library, so I can use it as a regular library.

This is what I have so far: https://gist.github.com/untidy-hair/9e432cb024c00dd8fa25ebcceda6a81a And this is the new test: https://gist.github.com/untidy-hair/563c9efa55f14108fbb2199d68534cc5

Whether Null comes at the beginning or at the end of the order is DB dependent. In that sense, my implementation there is a little bit opinionated for always putting NULL at the end (except when "last" exists). I personally cannot think of any reasons why NULL should come first but that can be configurable, too, if we like. (Caveat on README should be updated with this)

I was going to make a PR but first I thought it might be better asking here! Thanks in advance!

[EDIT] On a second thought, maybe it's easier to be able to see diffs than just the whole Gist file so I'm creating this PR as a draft for discussions πŸ™ #45

untidy-hair commented 1 year ago

Hi @thommor , are you still planning on releasing the new version? I'm guessing on this repo, commits like this and this will be necessary. Let me know if there's anything I can help with.

Also, I'm hoping #45 can resolve #18 , too. TIA! πŸ™‡

thommor commented 1 year ago

Hi @untidy-hair, yes sorry, work has been hectic the last few weeks.

I'll look into this this morning but I've realised I don't have PyPi access which isn't a good start.

untidy-hair commented 1 year ago

@thommor , thank you, I saw your PR. I appreciate you working on this while busy! Hopefully you can work on PyPI side smoothly, too :)

thommor commented 1 year ago

Yeah, release PR ready to go, just working on the PyPi side

patrick91 commented 1 year ago

@untidy-hair this has just been released as version 0.2.1 :)

untidy-hair commented 1 year ago

Thank you both very much!! It was great working with your team! Feel free to close this :)

thommor commented 1 year ago

You're welcome @untidy-hair, feel free to raise any other suggestions you may have in the future