-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Column alias expanding on ORDER BY #15302
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
b2a3c84
to
335b75a
Compare
Signed-off-by: Harshit Gangal <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
0d74fe4
to
cb1684d
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #15302 +/- ##
==========================================
+ Coverage 67.53% 67.64% +0.10%
==========================================
Files 1561 1561
Lines 193387 193535 +148
==========================================
+ Hits 130607 130913 +306
+ Misses 62780 62622 -158 ☔ View full report in Codecov by Sentry. |
…one order by path Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
|
||
func TestOrderByComplex(t *testing.T) { | ||
// tests written to try to trick the ORDER BY engine and planner | ||
utils.SkipIfBinaryIsBelowVersion(t, 20, "vtgate") |
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.
Should we change this once back ported? Since I think we need to back port this to v19 too since that has the previous PR and otherwise we have a significant regression?
Signed-off-by: Manan Gupta <[email protected]>
Co-authored-by: Harshit Gangal <[email protected]> Co-authored-by: Manan Gupta <[email protected]> Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Signed-off-by: Andres Taylor <[email protected]> Co-authored-by: Andrés Taylor <[email protected]> Co-authored-by: Harshit Gangal <[email protected]> Co-authored-by: Manan Gupta <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>
Description
In a recent PR, I did some work on how to rewrite column aliases used in ORDER BY, GROUP BY and HAVING.
I did not get that right, and introduced issues for a set of queries. This PR tries to correctly rewrite more column using the same rules that MySQL has.
Rewrite rules
Terminology first: an expression in the
SELECT
clause is a SELECT-expression. These can have aliased.An ORDER BY expression is an expression listed in the ORDER BY clause, without the ASC/DESC modifier.
Rewrite rule:
An unqualified column name in the ORDER BY that matches an alias among the SELECT expressions will be replaced with the SELECT expression if:
Related Issue(s)
Original PR #14935
Checklist
Deployment Notes