Make EnsureStableSortOrderBy respect the ordering direction for added fields#1560
Open
jbaehr wants to merge 1 commit intoOData:mainfrom
Open
Make EnsureStableSortOrderBy respect the ordering direction for added fields#1560jbaehr wants to merge 1 commit intoOData:mainfrom
jbaehr wants to merge 1 commit intoOData:mainfrom
Conversation
Previously, the fields added to the $orderby clause to enable stable
ordering were always in ascending order -- no matter what the original
sorting order was. This has led to two undesired side effects:
- As a user, I expect my results to be returned in reverse order if I
switch from asc to desc. Yet the order of items sharing the same value
for my original sorting criterion were not reversed.
- As a developer, I expect the same indices to be used (just traversed
in reverse order) of the sorting order from the user query is reversed.
Yet the generated query is not simply reversed, preventing the same
composite indices to be used. E.g. for MongoDB this currently requires
an additional index.
This commit addresses both issues by propagating the sorting order of
the last user-provided ordering criterion to all additional criteria
required to stabilize the ordering. So when the original query was
"$orderby=Foo" I got (and still get) an ordering of "Foo asc, Id asc".
Reversing this to "$orderby=Foo desc" I got "Foo desc, Id asc" and now
get "Foo desc, Id desc". This means an index on {Foo:1, Id:1} can be used
for both queries while previously an additional index on {Foo:-1, Id:1}
was required (example for MongoDB).
And for the user a result of "(A,1);(A,2); ... (Z,55);(Z,66)" reversed
is now "(Z,66);(Z55); ... (A,2);(A,1)" instead of the previous
"(Z55);(Z,66); ... (A,1);(A,2)"
Author
|
@microsoft-github-policy-service agree company="Rohde & Schwarz GmbH & Co. KG" |
Author
|
@xuzhg @gathogojr should I create a dedicated issue for the misbehavior fixed here or is the PR enough? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, the fields added to the $orderby clause to enable stable ordering were always in ascending order -- no matter what the original sorting order was. This has led to two undesired side effects:
This commit addresses both issues by propagating the sorting order of the last user-provided ordering criterion to all additional criteria required to stabilize the ordering. So when the original query was "$orderby=Foo" I got (and still get) an ordering of "Foo asc, Id asc". Reversing this to "$orderby=Foo desc" I previously got "Foo desc, Id asc" and now get "Foo desc, Id desc". This means an index on {Foo:1, Id:1} can be used for both queries while previously an additional index on {Foo:-1, Id:1} was required (example for MongoDB).
And for the user a result of "(A,1);(A,2); ... (Z,55);(Z,66)" reversed is now "(Z,66);(Z55); ... (A,2);(A,1)" as intuitively expected. Previously it was "(Z55);(Z,66); ... (A,1);(A,2)".