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

Bad SQL generation for common query pattern #2637

Closed
GSPP opened this issue Jul 16, 2015 · 3 comments
Closed

Bad SQL generation for common query pattern #2637

GSPP opened this issue Jul 16, 2015 · 3 comments

Comments

@GSPP
Copy link

GSPP commented Jul 16, 2015

See http://stackoverflow.com/questions/31458265/entity-framework-sum-case-not-optimized. The pattern presented there is a common query pattern to achieve a pivot. Since EF has no other way to generate a pivot this pattern should probably be made to work well.

I'm reproducing the key parts of the question here:

Here is the SQL I am trying to achieve (this is not what I'm getting):

    SELECT
        ClearingAccounts.ID,
        SUM(CASE WHEN Payments.StatusID = 1 THEN Payments.TotalAmount ELSE 0 END) AS Sum1,
        SUM(CASE WHEN DirectDebits.StatusID = 2 THEN DirectDebits.TotalAmount ELSE 0 END) AS Sum2,
        SUM(CASE WHEN Payments.StatusID = 2 THEN Payments.TotalAmount ELSE 0 END) AS Sum3,
        SUM(CASE WHEN DirectDebits.StatusID = 1 THEN DirectDebits.TotalAmount ELSE 0 END) AS Sum4
    FROM ClearingAccounts
    LEFT JOIN Payments ON Payments.ClearingAccountID = ClearingAccounts.ID
    LEFT JOIN DirectDebits ON DirectDebits.ClearingAccountID = ClearingAccounts.ID
    GROUP BY ClearingAccounts.ID

Here is the code:

    from clearingAccount in clearingAccounts
    let payments = clearingAccount.Payments
    let directDebits = clearingAccount.DirectDebits
    select new
    {
        ID = clearingAccount.ID,
        Sum1 = payments.Sum(p => p.StatusID == 1 ? p.TotalAmount : 0),
        Sum2 = directDebits.Sum(p => p.StatusID == 2 ? p.TotalAmount : 0),
        Sum3 = payments.Sum(p => p.StatusID == 2 ? p.TotalAmount : 0),
        Sum4 = directDebits.Sum(p => p.StatusID == 1 ? p.TotalAmount : 0),
    }

The is generated query:

    SELECT 
        [Project5].[ID] AS [ID], 
        [Project5].[C1] AS [C1], 
        [Project5].[C2] AS [C2], 
        [Project5].[C3] AS [C3], 
        [Project5].[C4] AS [C4]
        FROM ( SELECT 
            [Project4].[ID] AS [ID], 
            [Project4].[C1] AS [C1], 
            [Project4].[C2] AS [C2], 
            [Project4].[C3] AS [C3], 
            (SELECT 
                SUM([Filter5].[A1]) AS [A1]
                FROM ( SELECT 
                    CASE WHEN (1 = [Extent5].[StatusID]) THEN [Extent5].[TotalAmount] ELSE cast(0 as decimal(18)) END AS [A1]
                    FROM [dbo].[DirectDebits] AS [Extent5]
                    WHERE [Project4].[ID] = [Extent5].[ClearingAccountID]
                )  AS [Filter5]) AS [C4]
            FROM ( SELECT 
                [Project3].[ID] AS [ID], 
                [Project3].[C1] AS [C1], 
                [Project3].[C2] AS [C2], 
                (SELECT 
                    SUM([Filter4].[A1]) AS [A1]
                    FROM ( SELECT 
                        CASE WHEN (2 = [Extent4].[StatusID]) THEN [Extent4].[TotalAmount] ELSE cast(0 as decimal(18)) END AS [A1]
                        FROM [dbo].[Payments] AS [Extent4]
                        WHERE [Project3].[ID] = [Extent4].[ClearingAccountID]
                    )  AS [Filter4]) AS [C3]
                FROM ( SELECT 
                    [Project2].[ID] AS [ID], 
                    [Project2].[C1] AS [C1], 
                    (SELECT 
                        SUM([Filter3].[A1]) AS [A1]
                        FROM ( SELECT 
                            CASE WHEN (2 = [Extent3].[StatusID]) THEN [Extent3].[TotalAmount] ELSE cast(0 as decimal(18)) END AS [A1]
                            FROM [dbo].[DirectDebits] AS [Extent3]
                            WHERE [Project2].[ID] = [Extent3].[ClearingAccountID]
                        )  AS [Filter3]) AS [C2]
                    FROM ( SELECT 
                        [Project1].[ID] AS [ID], 
                        (SELECT 
                            SUM([Filter2].[A1]) AS [A1]
                            FROM ( SELECT 
                                CASE WHEN (1 = [Extent2].[StatusID]) THEN [Extent2].[TotalAmount] ELSE cast(0 as decimal(18)) END AS [A1]
                                FROM [dbo].[Payments] AS [Extent2]
                                WHERE [Project1].[ID] = [Extent2].[ClearingAccountID]
                            )  AS [Filter2]) AS [C1]
                        FROM ( SELECT 
                            [Extent1].[ID] AS [ID]
                            FROM [dbo].[ClearingAccounts] AS [Extent1]
                            WHERE ([Extent1].[CustomerID] = 3) AND ([Extent1].[Deleted] <> 1)
                        )  AS [Project1]
                    )  AS [Project2]
                )  AS [Project3]
            )  AS [Project4]
        )  AS [Project5]

I'm hoping that this is a minor change to the EF query generation pipeline. I do not propose specifically recognizing this pattern and doing something with it. Rather, I see no reason this SQL should not translate to something reasonable. Maybe there are some easy to address issues in code generation.

@ErikEJ
Copy link
Contributor

ErikEJ commented Jul 16, 2015

Is that not EF6 ?

@GSPP
Copy link
Author

GSPP commented Jul 16, 2015

That is EF6, yes. I forgot to mention. I request the change for EF7.

@rowanmiller
Copy link
Contributor

Hey,

This issue relates to EF6, which is managed in a separate repo (currently on CodePlex, but we are considering moving it to GitHub). We have taken a note on #2341 to consider this scenario and the SQL you suggested when we work on GroupBy translation in EF7.

Regarding this scenario in EF6, give that we are going to improve this in EF7 and the relative complexity/risk of adjusting the query pipeline in EF6 we aren't planning to address this in EF6. We would consider accepting a pull request though if someone outside our team wanted to contribute the change.

~Rowan

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
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

No branches or pull requests

4 participants