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

[6.0.2] Lift GroupByAggregate when correlation predicate matches #27108

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

smitpatel
Copy link
Member

@smitpatel smitpatel commented Jan 5, 2022

Earlier, we didn't match on exact predicate

Also fix bug in LikeExpression.Equals

Resolves #27102

Description

When projection after group by has an aggregation term which is not aggregate on grouping, incorrect SQL is generated which generates incorrect results or invalid SQL.

Customer impact

Customer queries will either give incorrect result or throw exception with invalid SQL.

How found

Customer reported on 6.0.1

Regression

Yes, From 5.0

Testing

Added test for user scenario which validates non grouping related aggregation.

Risk

Very low. Added quirk to revert to previous behavior.

@smitpatel smitpatel requested a review from a team January 5, 2022 03:49
@ajcvickers ajcvickers added this to the 6.0.x milestone Jan 6, 2022
@leecow leecow modified the milestones: 6.0.x, 6.0.2 Jan 6, 2022
Earlier, we didn't match on exact predicate

Also fix bug in LikeExpression.Equals

Resolves #27102
@smitpatel smitpatel merged commit 4907571 into release/6.0 Jan 10, 2022
@smitpatel smitpatel deleted the smit/newyearnewme branch January 10, 2022 19:07
@smitpatel smitpatel removed this from the 6.0.2 milestone Jan 10, 2022
@ajcvickers ajcvickers changed the title [release/6.0] Lift GroupByAggregate when correlation predicate matches [6.0.2] Lift GroupByAggregate when correlation predicate matches Jan 13, 2022
@AndriySvyryd
Copy link
Member

Looks like there is no such thing as a "Very low" risk fix in query, see #27427

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants