-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Optimize MIN/MAX over DISTINCT #34699
Conversation
252e4f6
to
61a58a7
Compare
SELECT DISTINCT [o].[OrderID] | ||
FROM [Orders] AS [o] | ||
) AS [o0] | ||
SELECT MAX([o].[OrderID]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these tests seem to be aimed at checking exactly this behavior; should I add something similar or are they adequate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They look good to me...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @ranma42! Can you just confirm that there are tests still exercising DISTINCT in both scenarios (e.g. with Count)?
SELECT DISTINCT [o].[OrderID] | ||
FROM [Orders] AS [o] | ||
) AS [o0] | ||
SELECT MAX([o].[OrderID]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They look good to me...
/// Drops DISTINCT operator on the selector of the <see cref="EnumerableExpression" />. | ||
/// </summary> | ||
/// <returns>The new expression with specified component updated.</returns> | ||
public virtual EnumerableExpression DropDistinct() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I'd rather have a single SetDistinct(bool) here (or similar) rather than two to keep the API surface tighter
61a58a7
to
6820eb6
Compare
Yes 👍
|
Thanks! |
When a value is computed as MAX or MIN of a subquery, DISTINCT has no effect and can be removed.
Fixes #34483.