OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
457 stars 157 forks source link

Expression tree for inherit access does unneccessary work #496

Open davhdavh opened 2 years ago

davhdavh commented 2 years ago

The expression tree for a select query does silly type checks:

public record Student(int Id, string Name, DateTime Time, int Version = 0) : Res(Id);

public abstract record Res(int Id);
.Lambda #Lambda1<System.Func`2[TestOData.Student,Microsoft.AspNetCore.OData.Query.Wrapper.SelectSome`1[TestOData.Student]]>(TestOData.Student $$it)
{
    .New Microsoft.AspNetCore.OData.Query.Wrapper.SelectSome`1[TestOData.Student](){
        Model = .Constant<Microsoft.AspNetCore.OData.Query.Container.LinqParameterContainer+TypedLinqParameterContainer`1[Microsoft.OData.Edm.IEdmModel]>(Microsoft.AspNetCore.OData.Query.Container.LinqParameterContainer+TypedLinqParameterContainer`1[Microsoft.OData.Edm.IEdmModel]).TypedProperty,
        Container = .New Microsoft.AspNetCore.OData.Query.Container.PropertyContainer+NamedPropertyWithNext0`1[System.Nullable`1[System.Int32]]()
        {
            Name = "Id",
            Value = .If ($$it .As TestOData.Res == null) {
                null
            } .Else {
                (System.Nullable`1[System.Int32])($$it .As TestOData.Res).Id
            },
            Next0 = .New Microsoft.AspNetCore.OData.Query.Container.AutoSelectedNamedProperty`1[System.Nullable`1[System.Int32]](){
                Name = "Version",
                Value = .If ($$it == null) {
                    null
                } .Else {
                    (System.Nullable`1[System.Int32])$$it.Version
                }
            }
        }
    }
}

$$it .As TestOData.Res is ALWAYS true, and the type of the value is always same in SelectSome. Therefore the entire inner access could be done as:


.Lambda #Lambda1<System.Func`2[TestOData.Student,Microsoft.AspNetCore.OData.Query.Wrapper.SelectSome`1[TestOData.Student]]>(TestOData.Student $$it)
{
    .New Microsoft.AspNetCore.OData.Query.Wrapper.SelectSome`1[TestOData.Student](){
        Model = .Constant<Microsoft.AspNetCore.OData.Query.Container.LinqParameterContainer+TypedLinqParameterContainer`1[Microsoft.OData.Edm.IEdmModel]>(Microsoft.AspNetCore.OData.Query.Container.LinqParameterContainer+TypedLinqParameterContainer`1[Microsoft.OData.Edm.IEdmModel]).TypedProperty,
        Container = .New Microsoft.AspNetCore.OData.Query.Container.PropertyContainer+NamedPropertyWithNext0`1[System.Nullable`1[System.Int32]]()
        {
            Name = "Id",
            Value = $$it.Id,
            Next0 = .New Microsoft.AspNetCore.OData.Query.Container.AutoSelectedNamedProperty`1[System.Nullable`1[System.Int32]](){
                Name = "Version",
                Value = $$it.Version
           }
        }
    }
}
xuzhg commented 2 years ago

@davhdavh Which version are you using?

If you are using 8.0.7, please set the NullPropagation=false, If you are using 8.0.8, It should be improved.

Looking forward to your feedback.

davhdavh commented 2 years ago

Was on 8.0.7, checked on 8.0.8 where it is exactly the same.

NullPropagation = false does help, but it still does unnecessary type casts: ($$it .As TestOData.ResourceDto).Name, or (System.Nullable``1[System.Int16])($$it .As TestOData.ResourceDto).Version. Also Version is not even nullable in my model