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: make null semantics optimizations provider-specific #18689

Closed
maumar opened this issue Oct 31, 2019 · 4 comments · Fixed by #19168
Closed

Query: make null semantics optimizations provider-specific #18689

maumar opened this issue Oct 31, 2019 · 4 comments · Fixed by #19168
Assignees
Labels
area-perf area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. providers-beware type-enhancement
Milestone

Comments

@maumar
Copy link
Contributor

maumar commented Oct 31, 2019

There are some null semantics optimizations we can do on SQL Server but not on other providers. We should consider making NullSemanticsRewritingExpressionVisitor provider specific.

example query:

ctx.Set<Weapon>().Select(
w => new
{
    w.Id,
    IsCartridge = !w.IsAutomatic && w.SynergyWithId == 1
})

null semantics rewriting visitor when initially encounters this expression determines that it cant do optimized translation, since its in the projection,
so we get:

!w.IsAutomatic && w.SynergyWithId == 1 && w.SynergyWithId != null

however, later on those conditions get converted to CASE blocks, in which optimized translation would be allowed.

If null semantics would be a provider specific, it could always assume it can optimize conditions in projection (and maybe other places?), since search condition visitor will convert them all to CASE blocks.

Another optimization:

ctx.NullSemanticsEntity1.Where(e => e.NullableBoolA == e.BoolB == (e.IntA == e.NullableIntB)));

on sql server we can apply optimized translation to the child comparisons, because they get converted to CASE blocks, on other providers we can't.

@maumar maumar changed the title Query: null semantics doesn't do optimized translation for predicates in projection on Sql Server, even though it could Query: make null semantics optimizations provider-specific Nov 1, 2019
@ajcvickers ajcvickers added this to the Backlog milestone Nov 1, 2019
@maumar
Copy link
Contributor Author

maumar commented Nov 1, 2019

Another optimization available on sql server:

true == NOT(a) -> NOT(a)

since NOT(a) on sql server (assuming we dont run relational null semantics) can never be null

@maumar
Copy link
Contributor Author

maumar commented Nov 1, 2019

removing propose-close, there is decent value in doing it in the future, imo

@maumar maumar self-assigned this Nov 8, 2019
@maumar maumar modified the milestones: Backlog, 5.0.0 Nov 14, 2019
maumar added a commit that referenced this issue Dec 5, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls.

Resolves #11464
Resolves #15722
Resolves #18597
Resolves #18689
maumar added a commit that referenced this issue Dec 5, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls.

Resolves #11464
Resolves #15722
Resolves #18597
Resolves #18689
maumar added a commit that referenced this issue Dec 5, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls.

Resolves #11464
Resolves #15722
Resolves #18597
Resolves #18689
maumar added a commit that referenced this issue Dec 5, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls.

Resolves #11464
Resolves #15722
Resolves #18597
Resolves #18689
maumar added a commit that referenced this issue Dec 5, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls.

Resolves #11464
Resolves #15722
Resolves #18597
Resolves #18689
maumar added a commit that referenced this issue Dec 5, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls.

Resolves #11464
Resolves #15722
Resolves #18597
Resolves #18689
maumar added a commit that referenced this issue Dec 5, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls.

Resolves #11464
Resolves #15722
Resolves #18597
Resolves #18689
maumar added a commit that referenced this issue Dec 5, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls.

Resolves #11464
Resolves #15722
Resolves #18597
Resolves #18689
maumar added a commit that referenced this issue Dec 5, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls.

Resolves #11464
Resolves #15722
Resolves #18597
Resolves #18689
maumar added a commit that referenced this issue Dec 13, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Dec 13, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
@smitpatel
Copy link
Contributor

@maumar to present design of this in next design meeting.

maumar added a commit that referenced this issue Dec 16, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Dec 16, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Dec 16, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Dec 18, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- moving optimizations that depend on knowing the nullability to NullSemantics visitor - optimizer now only contains optimizations that also work in 3-value logic, or when we know nulls can't happen,
- merging InExpressionValuesExpandingExpressionVisitor int NullSemantics visitor, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Dec 31, 2019
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Jan 1, 2020
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Jan 2, 2020
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Jan 4, 2020
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Jan 7, 2020
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
@smitpatel
Copy link
Contributor

@maumar - are you saying that fix to this issue is not needed? :trollface:

maumar added a commit that referenced this issue Jan 8, 2020
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Jan 8, 2020
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Jan 8, 2020
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Jan 8, 2020
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Jan 8, 2020
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
maumar added a commit that referenced this issue Jan 8, 2020
- moving NullSemantics visitor after 2nd level cache - we need to know the parameter values to properly handle IN expressions wrt null semantics,
- NullSemantics visitor needs to go before SqlExpressionOptimizer and SearchCondition, so those two are also moved after 2nd level cache,
- combining NullSemantics with SqlExpressionOptimizer (kept SqlExpressionOptimizer name) - SearchCondition now applies some relevant optimization itself, so that we don't need to run full optimizer afterwards,
- merging InExpressionValuesExpandingExpressionVisitor into SqlExpressionOptimizer as well, so that we don't apply the rewrite for UseRelationalNulls,
- preventing NulSemantics from performing double visitation when computing non-nullable columns.

Resolves #11464
Resolves #15722
Resolves #18338
Resolves #18597
Resolves #18689
Resolves #19019
@maumar maumar closed this as completed in a5f3e26 Jan 8, 2020
@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 Jan 9, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview1 Mar 13, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-preview1, 5.0.0 Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-perf area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. providers-beware type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants