Skip to content

Add null sorting#21

Merged
stephane merged 3 commits intowebstack:masterfrom
manderj:add_null_sorting
Mar 10, 2022
Merged

Add null sorting#21
stephane merged 3 commits intowebstack:masterfrom
manderj:add_null_sorting

Conversation

@manderj
Copy link
Contributor

@manderj manderj commented Mar 7, 2022

Hey 👋

I would like to contribute,
I had to make some changes for work and think it would be nice to have it merged.
the changes are quite simple and just consists of allowing sorting by null values (in first or last position) on the selected column.
I'll gladly answer any question

sorting by an invalid direction shouldn't fail and use ASC order
Adding nulls_first=True or nulls_last=True to autosort templatetags (or sort_queryset in case of jinja templates)
will sort the null values respectively on first and last places.
@stephane stephane merged commit 595eb01 into webstack:master Mar 10, 2022
@stephane
Copy link
Member

@manderj Could you submit another patch to handle nulls=first or nulls=last to avoid complexity that ensure current nulls_last=True and nulls_first=True are exclusives?

Another solution, would be to only handle nulls_first=True|False and to left it undefined when not set.
I think I prefer this solution.

By default, b-tree index of PostgreSQL uses nulls last but I think it's important to not explicitly set an ordering of null as you did.

@manderj
Copy link
Contributor Author

manderj commented Mar 10, 2022

@manderj Could you submit another patch to handle nulls=first or nulls=last to avoid complexity that ensure current nulls_last=True and nulls_first=True are exclusives?

Another solution, would be to only handle nulls_first=True|False and to left it undefined when not set.
I think I prefer this solution.

By default, b-tree index of PostgreSQL uses nulls last but I think it's important to not explicitly set an ordering of null as you did.

Okay 👍, I'll seen into that later today and will make a new pull request when I'm done 🙂
Thanks for the fast answer!

@stephane
Copy link
Member

Any news?

@manderj
Copy link
Contributor Author

manderj commented Apr 9, 2022

Hey, sorry I didn't make any update.
I'm currently hospitalized, whenever I'll be out I'll make the changes.
I'm every sorry for the inconveniences.

@stephane
Copy link
Member

Oh, don't worry. I hope you're fine, take time to recover.
Health is more important than code ;)

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

Successfully merging this pull request may close these issues.

2 participants