-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
chore(sqllab): Do not strip comments when executing SQL statements #27725
chore(sqllab): Do not strip comments when executing SQL statements #27725
Conversation
a05d7b9
to
0b35815
Compare
0b35815
to
7b317d6
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #27725 +/- ##
==========================================
+ Coverage 67.46% 69.85% +2.38%
==========================================
Files 1910 1911 +1
Lines 74802 74988 +186
Branches 8345 8345
==========================================
+ Hits 50467 52381 +1914
+ Misses 22284 20556 -1728
Partials 2051 2051
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Oh yeah, we should definitely not be stripping comments. There are multiple reasons to leave them — in addition to optimization like you mentioned I've also seen them used for auditing. Thanks for fixing this!
(I restarted the flaky MySQL test for you.) |
7b317d6
to
225a04d
Compare
225a04d
to
cdfa700
Compare
cdfa700
to
a04a63b
Compare
Following up in the footsteps of #27725 as comments can be used as a way to pass the database engine hints, or for logging purposes.
SUMMARY
I realize there is merit for stripping SQL comments when parsing SQL to extract tables etc., i.e., to aid with simplification, however I'm not entirely sure why we do this when executing SQL especially given that numerous SQL dialects supports optimizer hints in the form of comments:
This PR ensures when running SQL statements in SQL Lab we don't strip away the comments.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI.
ADDITIONAL INFORMATION