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

Query: SQL group by when aggregate operators have complex expression #11976

Closed
smitpatel opened this issue May 11, 2018 · 1 comment
Closed
Assignees
Labels
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

@smitpatel
Copy link
Member

        [ConditionalFact]
        public virtual void GroupBy_Property_Select_Sum()
        {
            AssertQueryScalar<Order>(os => os.GroupBy(o => o.CustomerID).Select(g => g.Sum(o => o.OrderID + 5)));
        }

Is client evaluated since the expression inside Sum is not member expression. That is our current logic to avoid issues with unmapped properties. In general we can relax the criteria but currently we decide in RequiresMaterialization if we can translate of not. At that point we cannot accurately know if we can translate it or not. (materialization tells client group by vs server group by). We can remove strict check and allow translation of more patterns but if something which cannot be translated is encountered then we fail.

As an effective fix for this (and other issue with joins #10012 ) we can pull the selector into GroupResultOperator (with custom operator) and we can make determination of group by during translation strictly.

@smitpatel
Copy link
Member Author

Dependent on #12089

@smitpatel smitpatel self-assigned this Jul 1, 2019
@smitpatel smitpatel modified the milestones: Backlog, 3.0.0 Jul 1, 2019
@smitpatel smitpatel added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed punted-for-3.0 labels Jul 1, 2019
smitpatel added a commit that referenced this issue Jul 2, 2019
…fter GroupBy

Resolves #12826
Resolves #6658
Part of #15711
Resolves #15853
Resolves #12799
Resolves #12476
Resolves #11976

There are way too many existing issues are resolved by this PR. I haven't added regression test or verified each of them so I have put Verify-Fixed label on them for now.
smitpatel added a commit that referenced this issue Jul 2, 2019
…fter GroupBy

Resolves #12826
Resolves #6658
Part of #15711
Resolves #15853
Resolves #12799
Resolves #12476
Resolves #11976

There are way too many existing issues are resolved by this PR. I haven't added regression test or verified each of them so I have put Verify-Fixed label on them for now.
smitpatel added a commit that referenced this issue Jul 2, 2019
…fter GroupBy

Resolves #12826
Resolves #6658
Part of #15711
Resolves #15853
Resolves #12799
Resolves #12476
Resolves #11976

There are way too many existing issues are resolved by this PR. I haven't added regression test or verified each of them so I have put Verify-Fixed label on them for now.
@smitpatel smitpatel modified the milestones: 3.0.0, 3.0.0-preview7 Jul 2, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview7, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

No branches or pull requests

3 participants