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

Support read-only collections of primitive types #31722

Closed
linkdotnet opened this issue Sep 13, 2023 · 7 comments · Fixed by #33456
Closed

Support read-only collections of primitive types #31722

linkdotnet opened this issue Sep 13, 2023 · 7 comments · Fixed by #33456
Labels
area-primitive-collections closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@linkdotnet
Copy link

File a bug

With the latest rc1, there is a regression if an object uses a IReadOnlyCollection<string> (or any other primitive type that can be mapped to JSON inside the column). The following code will throw exceptions, that were not existing on preview-7:

using Microsoft.EntityFrameworkCore;

using var context = new MyDbContext();
var allObjects = context.MyObjects.ToArray();

public class MyDbContext : DbContext
{
    public DbSet<MyObject> MyObjects { get; set; }

    public MyDbContext()
    {
        Database.EnsureCreated();
    }
    
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlite("Data Source=MyDatabase.db");
    }
}

public class MyObject
{
    public int Id { get; set; }
    
    public IReadOnlyCollection<string> Tags { get; set; }
}

Stack-Trace:

Unhandled exception. System.ArgumentException: Expression of type 'System.Collections.Generic.IReadOnlyCollection`1[System.String]' cannot be used for parameter of type 'System.Collections.Generic.IList`1[System.String]' of method 'System.Collections.Generic.IList`1[System.String] PopulateList[String](System.Collections.Generic.IList`1[System.String], System.Collections.Generic.IList`1[System.String])' (Parameter 'arg0')
   at System.Dynamic.Utils.ExpressionUtils.ValidateOneArgument(MethodBase method, ExpressionType nodeKind, Expression arguments, ParameterInfo pi, String methodParamName, String argumentParamName, Int32 index)
   at System.Linq.Expressions.Expression.Call(MethodInfo method, Expression arg0, Expression arg1)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityMaterializerSource.<AddInitializeExpressions>g__CreateMemberAssignment|12_0(Expression parameter, MemberInfo memberInfo, IPropertyBase property, Expression value)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityMaterializerSource.AddInitializeExpressions(HashSet`1 properties, ParameterBindingInfo bindingInfo, Expression instanceVariable, List`1 blockExpressions)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityMaterializerSource.CreateMaterializeExpression(List`1 blockExpressions, ParameterExpression instanceVariable, Expression constructorExpression, HashSet`1 properties, ParameterBindingInfo bindingInfo)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityMaterializerSource.CreateMaterializeExpression(EntityMaterializerSourceParameters parameters, Expression materializationContextExpression)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.EntityMaterializerInjectingExpressionVisitor.CreateFullMaterializeExpression(ITypeBase concreteTypeBase, ValueTuple`4 materializeExpressionContext)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.EntityMaterializerInjectingExpressionVisitor.MaterializeEntity(StructuralTypeShaperExpression shaper, ParameterExpression materializationContextVariable, ParameterExpression concreteEntityTypeVariable, ParameterExpression instanceVariable, ParameterExpression entryVariable)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.EntityMaterializerInjectingExpressionVisitor.ProcessEntityShaper(StructuralTypeShaperExpression shaper)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.EntityMaterializerInjectingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.EntityMaterializerInjectingExpressionVisitor.Inject(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.InjectEntityMaterializers(Expression expression)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.ProcessShaper(Expression shaperExpression, RelationalCommandCache& relationalCommandCache, IReadOnlyList`1& readerColumns, LambdaExpression& relatedDataLoaders, Int32& collectionId)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.VisitShapedQuery(ShapedQueryExpression shapedQueryExpression)
   at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at Microsoft.EntityFrameworkCore.Query.RelationalShapedQueryCompilingExpressionVisitor.VisitExtension(Expression extensionExpression)
   at Microsoft.EntityFrameworkCore.Query.QueryCompilationContext.CreateQueryExecutor[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Storage.Database.CompileQuery[TResult](Expression query, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.CompileQueryCore[TResult](IDatabase database, Expression query, IModel model, Boolean async)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass9_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Query.Internal.CompiledQueryCache.GetOrAddQuery[TResult](Object cacheKey, Func`1 compiler)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1.GetEnumerator()
   at Microsoft.EntityFrameworkCore.Internal.InternalDbSet`1.System.Collections.Generic.IEnumerable<TEntity>.GetEnumerator()
   at System.Collections.Generic.LargeArrayBuilder`1.AddRange(IEnumerable`1 items)
   at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source)
   at Program.<Main>$(String[] args) in /Users/stevengiesel/repos/BlogExamples/Repro/Repro/Program.cs:line 6

Include verbose output

The project is using net8.0-rc.1.

C:\Stuff\AllTogetherNow\FiveOh>dotnet ef dbcontext list --verbose
Using project 'C:\Stuff\AllTogetherNow\FiveOh\FiveOh.csproj'.
...
Finding DbContext classes in the project...
Found DbContext 'BlogContext'.
BlogContext

Include provider and version information

EF Core version:
Database provider: Microsoft.EntityFrameworkCore.Sqlite
Target framework:: .NET 8.0
Operating system: MacOS
IDE: -

@roji
Copy link
Member

roji commented Sep 13, 2023

Confirmed that the above is a regression from preview.7 to rc.1.

@ajcvickers @maumar is one of you interested in taking a look? Seems to be materialization-related.

@ajcvickers
Copy link
Contributor

@roji Did you try it on the latest daily build?

@roji
Copy link
Member

roji commented Sep 14, 2023

@ajcvickers yeah, sorry, I should have been clearer - this fails on the latest release/8.0 commit (a4db3b7) with the same exception.

@maumar
Copy link
Contributor

maumar commented Sep 15, 2023

culptit: #31272 EntityMaterializerSource -> CreateMemberAssignment

PopulateList is trying to add elements from buffer to the target, but that's not allowed for read only collections.

@ajcvickers
Copy link
Contributor

I missed this was a read-only collection. We don't support read-only collections in EF 8. We should throw a better exception. (They weren't fully functional in earlier EF8 previews, they just didn't throw.)

ajcvickers added a commit that referenced this issue Sep 15, 2023
@ajcvickers ajcvickers changed the title Regression in 8.0-rc.1 with primitive readonly lists Support read-only collections of primitive types Sep 15, 2023
@ajcvickers
Copy link
Contributor

See also #25364 when fixing this.

@ajcvickers ajcvickers added this to the Backlog milestone Sep 22, 2023
@ajcvickers ajcvickers self-assigned this Apr 1, 2024
@ajcvickers ajcvickers modified the milestones: Backlog, 9.0.0 Apr 1, 2024
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 2, 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 ajcvickers modified the milestones: 9.0.0, 9.0.0-preview4 Apr 8, 2024
@ajcvickers ajcvickers removed their assignment Aug 31, 2024
@roji roji modified the milestones: 9.0.0-preview4, 9.0.0 Oct 12, 2024
@Atomosk
Copy link

Atomosk commented Nov 11, 2024

I missed this was a read-only collection. We don't support read-only collections in EF 8. We should throw a better exception. (They weren't fully functional in earlier EF8 previews, they just didn't throw.)

But it throws the error even with List<> backing field and EntityTypeBuilder.Property().HasField().UsePropertyAccessMode(PropertyAccessMode.Field). Should't using field access mode make property declaration irrelevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-primitive-collections closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants