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

Allow users to control null sorting first/last #627

Open
jcachat opened this issue Sep 5, 2018 · 14 comments
Open

Allow users to control null sorting first/last #627

jcachat opened this issue Sep 5, 2018 · 14 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@jcachat
Copy link

jcachat commented Sep 5, 2018

I see support for this was added in #50 but the option to enable it is marked internal. I can't see any way for me to enable it. My users are complaining about null/empty values being sorted to the bottom of lists so this seems like the right way for me to fix it.

Are there plans to make this public?

@roji
Copy link
Member

roji commented Sep 6, 2018

@jcachat I implemented #50 only as a means of running the EF Core functional test suite, which asserts on results assuming that nulls are sorted first (like in C# LINQ). So it was meant purely as a testing feature, rather than something actual users would use.

Making this into an actual user-facing feature would involve more work, specifically thinking what to do about indices, which also support specifying null ordering. If the index's null ordering doesn't correspond to the query, a potentially severe performance degradation may occur.

I'm curious, can you provide more detail about the user complaints? Why are users expecting nulls to be sorted first rather than last? Sorting last is the PostgreSQL (default) behavior, and EF Core in general doesn't attempt to impose uniform behavior across databases, abstracting away differences such as this. So I'm interested in the reasoning your users give when they ask to change this.

@jcachat
Copy link
Author

jcachat commented Sep 6, 2018

Some of the columns I'm sorting and displaying have a mix of null and empty values. That's mostly because it's data imported from another system but also in some cases where there is a difference in a field that has been visited (and left empty) vs. a field that has not been touched at all (left as null). So it causes the empty values to be at the top of the list and the null values to be at the bottom. The null columns are displayed to the user as...nothing...so it just ends up looking wrong.

They are also converting from an application that used SQL Server which obviously sorts nulls first. So, they are expecting these lists & reports to be sorted the same way.

Where I really don't care about null vs empty, I can probably do something in the application and/or data migration to coalesce the values to be empty/0. But this is potentially affecting lots of tables & columns. And I'm not sure I can do that on some fields - especially numeric where the difference between null and 0 could be significant. So the option on the sort seems like the right solution.

I really wish PostgreSQL had a database level setting for this. Or did I miss that?

@roji
Copy link
Member

roji commented Sep 7, 2018

Some of the columns I'm sorting and displaying have a mix of null and empty values. That's mostly because it's data imported from another system but also in some cases where there is a difference in a field that has been visited (and left empty) vs. a field that has not been touched at all (left as null). So it causes the empty values to be at the top of the list and the null values to be at the bottom. The null columns are displayed to the user as...nothing...so it just ends up looking wrong.

That sounds more like a UI issue than something that needs to be fixed in the database layer... You can use coalescing (or just some client-side code) to convert nulls into whatever you want them to be shown as...

They are also converting from an application that used SQL Server which obviously sorts nulls first. So, they are expecting these lists & reports to be sorted the same way.

That's understandable, but unfortunately databases simply aren't that portable, and moving from one to another does imply certain changes. As a matter of principle, EF Core does not attempt to provide a single abstraction that behaves everywhere on all databases and makes code 100% portable.

Where I really don't care about null vs empty, I can probably do something in the application and/or data migration to coalesce the values to be empty/0. But this is potentially affecting lots of tables & columns. And I'm not sure I can do that on some fields - especially numeric where the difference between null and 0 could be significant. So the option on the sort seems like the right solution.

I'm not sure I understand - for numeric fields you wouldn't need to coalesce, because there's no such thing as empty (as opposed to null), right?

I really wish PostgreSQL had a database level setting for this. Or did I miss that?

Unfortunately not as far as I know... The only solution is to append NULLS FIRST to each and every ORDER BY clause and each and every CREATE INDEX statement.

Don't get me wrong, I do feel your pain. But the problems you're describing are better handled in the application. The danger of accidentally getting this wrong - running an ORDER BY ... NULLS FIRST where the index isn't configured in the same way - could be potentially disastrous performance-wise, we'd need to handle stuff like the nulls first option being added to an existing application with existing databases, and handle converting existing indexes from nulls last to nulls first... I'd really prefer to avoid this complexity.

@jcachat
Copy link
Author

jcachat commented Sep 7, 2018

That sounds more like a UI issue than something that needs to be fixed in the database layer... You can use coalescing (or just some client-side code) to convert nulls into whatever you want them to be shown as...

In some cases, that may be true. But my biggest pain point is with paged lists which are done using skip/take. So those sorts must be done in the database query.

I'm not sure I understand - for numeric fields you wouldn't need to coalesce, because there's no such thing as empty (as opposed to null), right?

I would just need to do something to get the null int values sorted to the top of the list as the users are expecting. Telling them "we're using a different database now so it does this differently" doesn't fly with them - they want what they want.

I completely understand everything you're saying and the complexities of implementing it. PostgreSQL is a very complicated database with lots of unique features so it's not reasonable to expect all of that to be exposed in EF Core (after all, it's not a PostgreSQL driver). But when I google-stumbled on to the referenced issue, I thought there was hope.

I really appreciate your time. I'll try to work through this from the application side. But...if you do find a way to get this one in there, I think it would be very helpful. Especially for those of us that are used to MS tools and SQL Server. And I'll volunteer as a tester.

@roji
Copy link
Member

roji commented Nov 14, 2018

Sorry for disappearing on this issue (got sucked into unrelated work), and thanks for understanding my reluctance to touch this.

As a general rule, it sounds like most of the problems described are better handled either by cleaning up data (i.e. converting empty strings to nulls or vice versa) or at the UI level. That isn't to say that I don't see any value in exposing NULLS FIRST in EF Core, but the risks and amount of work involved doesn't seem justified in this case...

I'll go ahead and close this issue for now. If other people are interested in this feature, please post back here, and if there's enough interest we'll definitely revisit.

@roji roji closed this as completed Nov 14, 2018
@rh78
Copy link

rh78 commented Jul 5, 2019

Hello I also support this issue. We're having the trouble that we want to sort by PRICE DESC. If we do that the NULLs come first. This is actually just not what the user expects. It should be flexible to choose how NULL handling should be done (FIRST or LAST).

It would be good to just enable that option and add a warning about the indices. We're fine with that.

@MolallaComm
Copy link

Moving from mysql/mssql to pgsql using aspnetcore odata - we've run into this also. Being able to just specify it in the DBContext for orderby would be enough for us also. At the same time I understand your hesitance to deal with it when in reality, it is IMHO a postgres problem. There should just be a config file option to set it globally for the server so as to make PG behave like other DBs do but my limited Google foo says there is no so option. I guess we can reimplement our views to return MinDate,EmptyString, etc. instead of null unless we hear an affirmative on this soonish.

@AshleyEke
Copy link

Is there any timescales on this being completed, I notice the option is there but still only internally accessible. Thanks

@roji
Copy link
Member

roji commented Jul 15, 2021

Given the comments and votes I'm reopening this to make it into a full user-facing feature.

Note to self: this should also affect index creation.

@roji roji reopened this Jul 15, 2021
@roji roji self-assigned this Jul 15, 2021
@roji roji added the enhancement New feature or request label Jul 15, 2021
@roji roji added this to the 6.0.0 milestone Jul 15, 2021
@roji roji changed the title ReverseNullOrdering / order by nulls first/last Allow users to control null sorting first/last Jul 15, 2021
@roji roji modified the milestones: 6.0.0, 7.0.0 Oct 9, 2021
@roji roji modified the milestones: 7.0.0, 8.0.0 Oct 15, 2022
@Akridian
Copy link

Akridian commented Sep 6, 2023

We are currently migrating from MS SQL Server to PostgreSQL and seeking any possibility to make logic as similar as possible.

Any update on this issue? Is it still considered for 8.0.0?

@roji
Copy link
Member

roji commented Sep 6, 2023

@Akridian this feature may still make it in for 8.0, but I can't say for sure - there's really a lot going on (and this isn't the most highly-voted issue either). I'd also recommend not trying to force PG to behave like MSSQL in general - you're not going to be able to do that anyway for many things.

@georg-jung
Copy link
Contributor

If somebody is blocked by this: Depending on the situation a simple workaround could be .OrderByDescending(x => x.Price.HasValue).ThenByDescending(x => x.Price) because false < true. See also this StackOverflow question & answer. Note that this does not emit NULLS FIRST or NULLS LAST in SQL but instead e.g. ORDER BY e.price IS NOT NULL DESC, e.price DESC, which might prevent usage of an index that could be used with e.g. NULLS LAST.

(@roji Not sure if it's worth the effort but if the hard part here is mainly the required API surface, it could be possible to detect this pattern and translate it to NULLS FIRST/LAST instead?)

@Akridian
Copy link

(@roji Not sure if it's worth the effort but if the hard part here is mainly the required API surface, it could be possible to detect this pattern and translate it to NULLS FIRST/LAST instead?)

Interesting suggestion, but it will not help in our case. We are migrating large system from MS SQL to PostgreSQL. And for the time, while we translate hundreds of stored procedures, we have to support both databases with C# code. Your approach will force us to introduce changes to C# codebase possibly impacting MS SQL performance.

I think NULLS order is a PostgreSQL feature that should be available to configure at least context-wise regardless of our case.

@roji
Copy link
Member

roji commented Sep 12, 2024

@Akridian and others, I do agree this should be supported (as it's a PG feature) - but it generally hasn't been high-priority enough and there have been other, more important things to implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants