hazzik / DelegateDecompiler

A library which is able to decompile a delegate or a method body to its lambda representation
MIT License
522 stars 62 forks source link

Decompiled IQueryable produces unoptimized SQL in some cases #149

Closed BrandoCaserotti closed 1 year ago

BrandoCaserotti commented 5 years ago

The following Query:

_context.Set<Fru>().AsNoTracking().Decompile().Select(fru => new { fru.Code, fru.AssociatedCants }).OrderBy(fru => fru.AssociatedCants).ToList();

produces the following SQL which could be optimized:

SELECT [fru].[Code], (
    SELECT COUNT(*)
    FROM [FruCants] AS [f0]
    WHERE [fru].[Id] = [f0].[FruId]
) AS [AssociatedCants]
FROM [Frus] AS [fru]
ORDER BY (
    SELECT COUNT(*)
    FROM [FruCants] AS [f]
    WHERE [fru].[Id] = [f].[FruId]
)

The property fru.AssociatedCants is a computed property over a 1-N navigation property:

[Computed]
public int AssociatedCants
{
    get => FruCants.Count();
}

According to the execution plan produced by SQL Server Manager the query could be rewritten this way :

SELECT [fru].[Code], (
    SELECT COUNT(*)
    FROM [FruCants] AS [f0]
    WHERE [fru].[Id] = [f0].[FruId]
) AS [AssociatedCants]
FROM [Frus] AS [fru]
ORDER BY ([AssociatedCants])

Is it an issue of EntityFramework Core? The Execution plan costs much more without the use of the alias ([AssociatedCans]), which includes many joins and index scans than the optimized plan.

BrandoCaserotti commented 5 years ago

The same happens if you filter against the computed property ( no alias for the column is used and the where clause is evaluated again):

SELECT TOP(10) [obj].[Code] AS [I0], (
    SELECT COUNT(*)
    FROM [FruCants] AS [f1]
    WHERE [obj].[Id] = [f1].[FruId]
) AS [I1], [obj].[Id] AS [I2]
FROM [Frus] AS [obj]
WHERE (
    SELECT COUNT(*)
    FROM [FruCants] AS [f]
    WHERE [obj].[Id] = [f].[FruId]
) = 0
ORDER BY (
    SELECT COUNT(*)
    FROM [FruCants] AS [f0]
    WHERE [obj].[Id] = [f0].[FruId]
) DESC
magicmoux commented 5 years ago

As far as I understand, the issue could comes from DD in that the expression tree seems to reference the memberaccess as still belonging to the Fru class instead of to an anonymous type whose member AssociatedCants should not be decompilable.

As it is, I'm not sure whether the fix should be handled in Processor.cs or in ExpressionVisitor.cs I'll try to look into that one if I go some free time but any help would be welcome in the meantime :) .

Max.

magicmoux commented 4 years ago

After a first simple check , it seems my first guess was wrong ; the issue does not come form DD core decompilation : Anonymous types' memberAccess seem to be preserved as expected.

So the issue could be specific to EF/EFCore optimization. As it is your code does not seems to meet the warnings raised By Hazzik in the documentation

Could you please try running this query and tell us whether the result is the same or not ?

_context.Set<Fru>().Select(fru => new { fru.Code, fru.AssociatedCants }).OrderBy(fru => fru.AssociatedCants).AsNoTracking().Decompile().ToList();

A plus would be to provide the debug view of the decompiled query to tell for sure whether the problem comes from DD or from EFCore.

Thx, Max

BrandoCaserotti commented 4 years ago

Hi magicmoux,

the generated SQL is the following : SELECT [fru].[Code], ( SELECT COUNT(*) FROM [FruCants] AS [f0] WHERE [fru].[Id] = [f0].[FruId] ) AS [AssociatedCants] FROM [Frus] AS [fru] ORDER BY ( SELECT COUNT(*) FROM [FruCants] AS [f] WHERE [fru].[Id] = [f].[FruId]

this is the DebugView of the decompiled IQueryable:

.Call Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.AsNoTracking(
    .Call System.Linq.Queryable.OrderBy(
        .Call System.Linq.Queryable.Select(
.Constant<Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[PowerWeb.Domain.Entities.Fru]>(Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryable`1[PowerWeb.Domain.Entities.Fru]),
                '(.Lambda #Lambda1<System.Func`2[PowerWeb.Domain.Entities.Fru,<>f__AnonymousType27`2[System.String,System.Int32]]>)),
                '(.Lambda #Lambda2<System.Func`2[<>f__AnonymousType27`2[System.String,System.Int32],System.Int32]>)))

                .Lambda #Lambda1<System.Func`2[PowerWeb.Domain.Entities.Fru,<>f__AnonymousType27`2[System.String,System.Int32]]>(PowerWeb.Domain.Entities.Fru $fru)
                {
                    .New <>f__AnonymousType27`2[System.String,System.Int32](
                        $fru.Code,
                            .Call System.Linq.Enumerable.Count($fru.FruCants))
                }
                .Lambda #Lambda2<System.Func`2[<>f__AnonymousType27`2[System.String,System.Int32],System.Int32]>(<>f__AnonymousType27`2[System.String,System.Int32] $fru)
                {
                    $fru.AssociatedCants
                }

Sorry for the bad indentation but the char " ` " of the generic type messes with the code snippet of github. If you need other info I would be glad to help.

Thank you for your time

magicmoux commented 4 years ago

Hi @zap93

Here come the bad news : the issue comes from EF Core. The following code seems to be translated into the same unoptimized query.

_context.Set<Fru>().AsNoTracking().Select(fru => new { fru.Code, AssociatedCantsCount = fru.AssociatedCants.Count }).OrderBy(fru => fru.AssociatedCantsCount ).ToList()

My bad, I should have checked this first

BrandoCaserotti commented 4 years ago

Good to know that the issue is not from DD. This library should become the standard for EF Core.

I will open a issue on EF Core github.

Thank you

hazzik commented 1 year ago

Closing as external issue. As far as I can see -- DD is doing exactly as it should.