[GitHub] [incubator-superset] dflionis opened a new issue #7483: Forced LIMIT clause not being applied correctly when SQL has a trailing comment

[GitBox <gi...@apache.org>]

dflionis opened a new issue #7483: Forced LIMIT clause not being applied correctly when SQL has a trailing comment
URL: https://github.com/apache/incubator-superset/issues/7483
 
 
   When a user submits a query that ends in a comment, Superset incorrectly appends its built-in `LIMIT` clause to that same last line which makes the `LIMIT` clause commented out as well.
   
   ### Expected results
   
   Ideally the limit would be added on a new line so that SQL comments cannot have any effect on it.
   
   ### Actual results
   
   Superset appends the `LIMIT` clause to the same line which is functionally equivalent to not adding a limit at all.
   
   #### Screenshots
   
   User submits the following query (with a default LIMIT of 500 in the UI set):
   ```sql
   SELECT y, m, d, COUNT(*)
   FROM some_table
   GROUP BY y, m, d -- uh oh a trailing comment!
   ```
   
   Superset submits to the database engine:
   ```sql
   SELECT y, m, d, COUNT(*)
   FROM some_table
   GROUP BY y, m, d -- uh oh a trailing comment! LIMIT 500
   ```
   
   #### How to reproduce the bug
   
   Run a query that ends in a trailing comment, such as the example user submission in the Screenshots section above.
   
   ### Environment
   
   - superset version: 0.32.0rc2
   - python version: 3.6
   - node.js version: ???
   - npm version: ???
   
   ### Checklist
   
   Make sure these boxes are checked before submitting your issue - thank you!
   
   - [x] I have checked the superset logs for python stacktraces and included it here as text if there are any.
   - [x] I have reproduced the issue with at least the latest released version of superset.
   - [x] I have checked the issue tracker for the same issue and I haven't found one similar.
   
   ### Additional context
   
   I'm not sure if its as simple as inserting a newline [here](https://github.com/apache/incubator-superset/blob/master/superset/sql_parse.py#L171) but I'm having trouble getting the tests to pass locally so I didn't want to submit a PR that I'm not confident about.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org