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

Stable ordering should be applied when using the EDM model for paging (in addition to EnableQueryAttribute.PageSize) #1018

Merged
merged 3 commits into from
Oct 11, 2023
Merged

Stable ordering should be applied when using the EDM model for paging (in addition to EnableQueryAttribute.PageSize) #1018

merged 3 commits into from
Oct 11, 2023

Conversation

dmitry-chistyakov
Copy link
Contributor

We use server-side paging feature and encountered an issue where OData applies stable ordering to the output dataset for $skip, $top, and EnableQueryAttribute.PageSize.

However, OData overlooks scenarios in which paging is defined through the EDM model, even though stable ordering logic should also be applied in such cases.

Although EnableQueryAttribute.PageSize is useful, it can become unwieldy in specific situations, especially when it propagates throughout all internal collections. This can lead to unexpected navigation through internal structures and retrieval of missing data. Additionally, issues may arise when constructing the @odata.nextLink for contained collections.

… (in addition to EnableQueryAttribute.PageSize).
@dmitry-chistyakov
Copy link
Contributor Author

@microsoft-github-policy-service agree company="Intermedia"

@ElizabethOkerio
Copy link
Contributor

ElizabethOkerio commented Aug 29, 2023

@dmitry-chistyakov please add tests

@gathogojr
Copy link
Contributor

Thank you @dmitry-chistyakov for your contribution. Could you please add tests to validate your change?

@dmitry-chistyakov
Copy link
Contributor Author

Thank you @dmitry-chistyakov for your contribution. Could you please add tests to validate your change?

Done. Please check.

@dmitry-chistyakov
Copy link
Contributor Author

@dmitry-chistyakov please add tests

Done. Please check.

Copy link
Contributor

@gathogojr gathogojr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@gathogojr gathogojr merged commit 89a1848 into OData:main Oct 11, 2023
2 checks passed
@gathogojr
Copy link
Contributor

Thank you @dmitry-chistyakov for your contribution

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.

4 participants