-
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
Fix aliasing in queries by keeping required projections #15943
Conversation
Signed-off-by: Manan Gupta <[email protected]>
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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15943 +/- ##
==========================================
+ Coverage 68.41% 68.43% +0.02%
==========================================
Files 1562 1562
Lines 197056 197052 -4
==========================================
+ Hits 134807 134850 +43
+ Misses 62249 62202 -47 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Manan Gupta <[email protected]>
Is this a bug that only exists on main aka v20? Are any back ports needed? |
I have replicated this bug on V19. |
We need to backport this all the way to V17 |
I checked at it doesn't help to back port this change to any of the releases. It is actually correct in the other releases. We introduced the capability to change the column names in simple projection in #15384, before that removing the simple projection if it was only passing through the columns was actually correct. |
Description
This PR fixes the issue pointed out in #15944.
The problem was that we were removing some Projection operators that were only passing through the columns in the same order that they were receiving in. However, they were changing the column aliases. These projections are still required and should not be removed.
Related Issue(s)
Checklist
Deployment Notes