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

Fix ordering the keys according to the ColumnAttribute.Order property #1867

Merged

Conversation

KanishManuja-MS
Copy link
Contributor

@KanishManuja-MS KanishManuja-MS commented Jul 29, 2019

This pull request fixes issue #1085

Description

In order to ensure a stable ordering of the results with different request, the library adds a default order when required.
When an entity has a composite key, the order of the properties matters, and if not specified correctly, the query may have bad performances.

The fix ensures that the order specified in the model via the ColumnAttribute.Order property is respected when the stable order is ensured by the library.
It does it by configuring the keys in the correct order in the EDM model, and not in the function applying the default order.

Also, previously the order of the key in the model was arbitrarely chosen, which could be suprising for the user when the default order was applied because it would not match what it in the model. Now the order in the model will be the same as the default order.

Side effect

A side effect is that the fix will may change the order to the key in the EDM model.

Checklist (Uncheck if it is not completed)

  • Test cases added
  • Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

Update ColumnAttributeEdmPropertyConvention.cs
@KanishManuja-MS KanishManuja-MS changed the title Duplicate - Fix ordering the keys according to the ColumnAttribute.Order property Fix ordering the keys according to the ColumnAttribute.Order property Jul 31, 2019
@KanishManuja-MS KanishManuja-MS merged commit bea7d89 into OData:master Jul 31, 2019
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.

None yet

2 participants