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

Use EF8-style primitive collections in the Cosmos provider #25364

Closed
Tracked by #30731 ...
AndriySvyryd opened this issue Jul 29, 2021 · 8 comments · Fixed by #33456
Closed
Tracked by #30731 ...

Use EF8-style primitive collections in the Cosmos provider #25364

AndriySvyryd opened this issue Jul 29, 2021 · 8 comments · Fixed by #33456
Labels
area-change-tracking area-cosmos area-primitive-collections closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-8.0 Originally planned for the EF Core 8.0 (EF8) release, but moved out due to resource constraints. type-enhancement

Comments

@AndriySvyryd
Copy link
Member

These could be useful for other providers, not just Cosmos: ListComparer, NullableListComparer, SingleDimensionalArrayComparer, NullableSingleDimensionalArrayComparer, StringDictionaryComparer, NullableStringDictionaryComparer

@roji
Copy link
Member

roji commented Jul 31, 2021

When doing this, decide on how to deal with readonly collections (see #25344 (comment)).

@AndriySvyryd
Copy link
Member Author

When doing this, decide on how to deal with readonly collections

Consider read-only collections of mutable types (nested collections)

@roji
Copy link
Member

roji commented Aug 1, 2021

Another provider that seems to support arrays which could benefit from this: https://github.com/cloudspannerecosystem/dotnet-spanner-entity-framework

@AndriySvyryd
Copy link
Member Author

Related to #17471

@roji
Copy link
Member

roji commented Apr 24, 2023

Design decision: we need to as part of the relational primitive collections work (#30731). Collection properties will use the value converter by default; where not appropriate (because the collection is huge), users can disable it.

Note: special-case .NET immutable collections such as ImmutableList; for these we don't need to snapshot, and the collection only changed if the reference changed (these types are particularly well-suited for change tracking).

roji added a commit to roji/efcore that referenced this issue May 20, 2023
Necessary for primitive collection, where e.g. the comparer has type
IList<int> but the type mapping is for the concrete List<int>.

Related to/part of dotnet#25364
@roji
Copy link
Member

roji commented May 20, 2023

Note: see the new implementation of the collection comparers in 8.0.0-preview4. Note that this restricts to IList only, and not to ISet as discussed in the design meeting, because of the difficulty to (efficiently) compare two sets using (EF's) element comparer.

ghost pushed a commit that referenced this issue May 22, 2023
Necessary for primitive collection, where e.g. the comparer has type
IList<int> but the type mapping is for the concrete List<int>.

Related to/part of #25364
@ajcvickers
Copy link
Member

Note for triage: #31480 added comparers for primitive collections, but these don't yet support nested collections. We should re-work them to do so, and then the Cosmos comparers can be removed.

@ajcvickers ajcvickers removed this from the 8.0.0 milestone Aug 17, 2023
@ajcvickers ajcvickers added the punted-for-8.0 Originally planned for the EF Core 8.0 (EF8) release, but moved out due to resource constraints. label Sep 6, 2023
@ajcvickers ajcvickers added this to the Backlog milestone Sep 6, 2023
@ajcvickers ajcvickers changed the title Move collection comparers to Core Use EF8-style primitive collections in the Cosmos provider Sep 26, 2023
@AndriySvyryd
Copy link
Member Author

Related to #4179

@ajcvickers ajcvickers modified the milestones: Backlog, 9.0.0 Apr 1, 2024
@ajcvickers ajcvickers added area-cosmos closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. labels Apr 1, 2024
ajcvickers added a commit that referenced this issue Apr 2, 2024
This change starts using the EF8 primitive collection infrastructure for primitive collections in Cosmos. This involves adding support for nested collections and read-only collections.

The main challenge here is supporting all the different collection types when, for example, `List<List<string>>` cannot be cast to `IEnumerable<IEnumerable<string>>`. To support this, we use exact collection types in snapshots, and we use non-generic methods in the nested comparers and serializers.

General dictionary mapping still not supported.
No extensive query testing done yet.

Model building and change tracking for #30713
Fixes #25364
Fixes #25343
Part of #4179
Fixes #31722
ajcvickers added a commit that referenced this issue Apr 3, 2024
This change starts using the EF8 primitive collection infrastructure for primitive collections in Cosmos. This involves adding support for nested collections and read-only collections.

The main challenge here is supporting all the different collection types when, for example, `List<List<string>>` cannot be cast to `IEnumerable<IEnumerable<string>>`. To support this, we use exact collection types in snapshots, and we use non-generic methods in the nested comparers and serializers.

General dictionary mapping still not supported.
No extensive query testing done yet.

Model building and change tracking for #30713
Fixes #25364
Fixes #25343
Part of #4179
Fixes #31722
ajcvickers added a commit that referenced this issue Apr 3, 2024
* Consolidate primitive collections across relational and Cosmos

This change starts using the EF8 primitive collection infrastructure for primitive collections in Cosmos. This involves adding support for nested collections and read-only collections.

The main challenge here is supporting all the different collection types when, for example, `List<List<string>>` cannot be cast to `IEnumerable<IEnumerable<string>>`. To support this, we use exact collection types in snapshots, and we use non-generic methods in the nested comparers and serializers.

General dictionary mapping still not supported.
No extensive query testing done yet.

Model building and change tracking for #30713
Fixes #25364
Fixes #25343
Part of #4179
Fixes #31722

* Remove dead files

* Updates based on review feedback
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview4 Apr 8, 2024
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-change-tracking area-cosmos area-primitive-collections closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-8.0 Originally planned for the EF Core 8.0 (EF8) release, but moved out due to resource constraints. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants