Skip to content

Conversation

@alexmo16
Copy link
Contributor

Changes

  • Add possibility of disabling stable ordering.
  • Add possibility of disabling default order by clause.

Why

Because in our application we need to disable EnsureStableOrdering in a specific query to improve performance.

- Add possibility of disabling default order by clause.
@BlaiseD
Copy link
Member

BlaiseD commented Apr 12, 2025

AlwaysSortByPrimaryKey is false by default. EnsureStableOrdering is true by default. Both address sorting.

I propose staying with AlwaysSortByPrimaryKey. Use AlwaysSortByPrimaryKey to address the new logic and set it to true for your use case.

Or what am I missing?

@alexmo16
Copy link
Contributor Author

Thanks for the quick feedback.

FYI, I added EnsureStableOrdering because it is an already existing property of the OData object ODataQuerySettings.

But I'll try your suggestion.

…aryKey

To fix the problem, I fix the code where ordering by primary key was added even when AlwaysSortByPrimaryKey was disabled.
@alexmo16
Copy link
Contributor Author

alexmo16 commented Apr 14, 2025

@BlaiseD I followed your suggestion and it seems to work and there is less changed code.

The only thing that I'm not sure is that by using AlwaysSortByPrimaryKey we are changing the default behavior.
Currently, when using a $top the SQL query contains an order by even when AlwaysSortByPrimaryKey is set to false, and now I'm changing that.
Maybe it's fine, but I want to double check that with you.

@BlaiseD
Copy link
Member

BlaiseD commented Apr 19, 2025

@BlaiseD I followed your suggestion and it seems to work and there is less changed code.

The only thing that I'm not sure is that by using AlwaysSortByPrimaryKey we are changing the default behavior. Currently, when using a $top the SQL query contains an order by even when AlwaysSortByPrimaryKey is set to false, and now I'm changing that. Maybe it's fine, but I want to double check that with you.

Yep - looks like an oversight - and I agree EnsureStableOrdering would have been a better property name for consistency.

@BlaiseD BlaiseD merged commit 32a8ee5 into AutoMapper:master Apr 19, 2025
3 checks passed
@amorelIM
Copy link

@BlaiseD Could you please provide an ETA for the next release? This will help me plan accordingly with my team.

@BlaiseD
Copy link
Member

BlaiseD commented Apr 23, 2025

7.0.1

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.

3 participants