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: Apply CompositePredicateExpressionVisitor to join predicates #9395

Closed
smitpatel opened this issue Aug 12, 2017 · 4 comments
Closed

Query: Apply CompositePredicateExpressionVisitor to join predicates #9395

smitpatel opened this issue Aug 12, 2017 · 4 comments
Assignees
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@smitpatel
Copy link
Contributor

At present it is applied to SelectExpression.Predicate only

@maumar
Copy link
Contributor

maumar commented Aug 12, 2017

Also add more optimizations, e.g. ((c.Discriminator == "Foo" ? c.FK : null) == o.PK into c.Discriminator == "Foo" && c.FK == o.PK)

@ajcvickers ajcvickers added this to the 2.1.0 milestone Aug 14, 2017
maumar added a commit that referenced this issue Aug 18, 2017
…in predicates

Some optimizations that are applied to predicates can be safely applied to join predicates as well. Fix modifies CompositePredicateExpressionVisitor to also look into JoinExpression predicates and not only SelectExpression predicates.
Also changes the way we build join predicates themselves, when inheritance is concerned - now the discriminator predicate will be extracted out of the comparison, so that the comparisons are sargable.

Before:

((a is Derived) ? a.DerivedFK : null) == b.PK - which translated to CASE block (not sargable)

After:

a is Derived && a.DerivedFK == b.PK
maumar added a commit that referenced this issue Aug 18, 2017
…in predicates

Some optimizations that are applied to predicates can be safely applied to join predicates as well. Fix modifies CompositePredicateExpressionVisitor to also look into JoinExpression predicates and not only SelectExpression predicates.
Also changes the way we build join predicates themselves, when inheritance is concerned - now the discriminator predicate will be extracted out of the comparison, so that the comparisons are sargable.

Before:

((a is Derived) ? a.DerivedFK : null) == b.PK - which translated to CASE block (not sargable)

After:

a is Derived && a.DerivedFK == b.PK
maumar added a commit that referenced this issue Aug 23, 2017
…in predicates

Some optimizations that are applied to predicates in SelectExpression can be safely applied to join predicates as well. Fix modifies CompositePredicateExpressionVisitor to also look into JoinExpression predicates and not only SelectExpression predicates.
Also changes the way we build join predicates themselves, when inheritance is concerned - now the discriminator predicate will be extracted out of the comparison, so that the comparisons are sargable.

Before:

((a is Derived) ? a.DerivedFK : null) == b.PK - which translated to CASE block (not sargable)

After:

a is Derived && a.DerivedFK == b.PK
maumar added a commit that referenced this issue Aug 24, 2017
…in predicates

Some optimizations that are applied to predicates in SelectExpression can be safely applied to join predicates as well. Fix modifies CompositePredicateExpressionVisitor to also look into JoinExpression predicates and not only SelectExpression predicates.
Also changes the way we build join predicates themselves, when inheritance is concerned - now the discriminator predicate will be extracted out of the comparison, so that the comparisons are sargable.

Before:

((a is Derived) ? a.DerivedFK : null) == b.PK - which translated to CASE block (not sargable)

After:

a is Derived && a.DerivedFK == b.PK
@ajcvickers ajcvickers modified the milestones: 2.1.0-preview1, 2.1.0 Jan 17, 2018
@ajcvickers
Copy link
Contributor

@maumar @smitpatel Please update description with more details and investigate for 2.1.

@smitpatel
Copy link
Contributor Author

Derived Include generates join condition using ternary to appropriately cast to derived type.
Which generates query like this

SELECT [l].[Name], [l].[Discriminator], [l].[LocustHordeId], [l].[ThreatLevel], [l].[DefeatedByNickname], [l].[DefeatedBySquadId], [t].[Nickname], [t].[SquadId], [t].[AssignedCityName], [t].[CityOrBirthName], [t].[Discriminator], [t].[FullName], [t].[HasSoulPatch], [t].[LeaderNickname], [t].[LeaderSquadId], [t].[Rank]
FROM [LocustLeaders] AS [l]
LEFT JOIN (
    SELECT [l.DefeatedBy].*
    FROM [Gears] AS [l.DefeatedBy]
    WHERE [l.DefeatedBy].[Discriminator] IN (N'Officer', N'Gear')
) AS [t] ON (CASE
    WHEN [l].[Discriminator] = N'LocustCommander'
    THEN [l].[DefeatedByNickname] ELSE NULL
END = [t].[Nickname]) AND (CASE
    WHEN [l].[Discriminator] = N'LocustCommander'
    THEN [l].[DefeatedBySquadId] ELSE NULL
END = [t].[SquadId])
WHERE [l].[Discriminator] IN (N'LocustCommander', N'LocustLeader')

Here using Case block on join condition means join cannot use any index and no longer sargable.

maumar added a commit that referenced this issue Feb 22, 2018
…in predicates

Some optimizations that are applied to predicates in SelectExpression can be safely applied to join predicates as well. Fix modifies CompositePredicateExpressionVisitor to also look into JoinExpression predicates and not only SelectExpression predicates.
Also changes the way we build join predicates themselves, when inheritance is concerned - now the discriminator predicate will be extracted out of the conditional expression, so that the comparisons are sargable.

Before:

((a is Derived) ? a.DerivedFK : null) == b.PK - which translated to CASE block (not sargable)

After:

a is Derived && a.DerivedFK == b.PK
maumar added a commit that referenced this issue Feb 22, 2018
…in predicates

Some optimizations that are applied to predicates in SelectExpression can be safely applied to join predicates as well. Fix modifies CompositePredicateExpressionVisitor to also look into JoinExpression predicates and not only SelectExpression predicates.
Also changes the way we build join predicates themselves, when inheritance is concerned - now the discriminator predicate will be extracted out of the conditional expression, so that the comparisons are sargable.

Before:

((a is Derived) ? a.DerivedFK : null) == b.PK - which translated to CASE block (not sargable)

After:

a is Derived && a.DerivedFK == b.PK
maumar added a commit that referenced this issue Feb 23, 2018
…in predicates

Some optimizations that are applied to predicates in SelectExpression can be safely applied to join predicates as well. Fix modifies CompositePredicateExpressionVisitor to also look into JoinExpression predicates and not only SelectExpression predicates.
Also changes the way we build join predicates themselves, when inheritance is concerned - now the discriminator predicate will be extracted out of the conditional expression, so that the comparisons are sargable.

Before:

((a is Derived) ? a.DerivedFK : null) == b.PK - which translated to CASE block (not sargable)

After:

a is Derived && a.DerivedFK == b.PK
maumar added a commit that referenced this issue Feb 23, 2018
…in predicates

Some optimizations that are applied to predicates in SelectExpression can be safely applied to join predicates as well. Fix modifies CompositePredicateExpressionVisitor to also look into JoinExpression predicates and not only SelectExpression predicates.
Also changes the way we build join predicates themselves, when inheritance is concerned - now the discriminator predicate will be extracted out of the conditional expression, so that the comparisons are sargable.

Before:

((a is Derived) ? a.DerivedFK : null) == b.PK - which translated to CASE block (not sargable)

After:

a is Derived && a.DerivedFK == b.PK
@maumar
Copy link
Contributor

maumar commented Feb 23, 2018

fixed in 9355f64

@maumar maumar closed this as completed Feb 23, 2018
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Feb 23, 2018
@ajcvickers ajcvickers modified the milestones: 2.1.0-preview2, 2.1.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. type-enhancement
Projects
None yet
Development

No branches or pull requests

3 participants