[GitHub] john-bodley commented on a change in pull request #5295: [sqllab] Fix sqllab limit regex issue with sqlparse

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

john-bodley commented on a change in pull request #5295: [sqllab] Fix sqllab limit regex issue with sqlparse
URL: https://github.com/apache/incubator-superset/pull/5295#discussion_r199312889
 
 

 ##########
 File path: superset/sql_parse.py
 ##########
 @@ -128,3 +134,46 @@ def __extract_from_token(self, token):
                 for token in item.tokens:
                     if self.__is_identifier(token):
                         self.__process_identifier(token)
+
+    def _get_limit_from_token(self, token):
+        value = token.value
+        if ',' in value:
+            return int(value.split(',')[0])
+        else:
+            return int(value)
+
+    def _extract_limit_from_outermost_layer(self, statement):
+        limit_token = None
+        for pos, item in enumerate(statement.tokens):
+            if item.ttype in Keyword and item.value.lower() == 'limit':
+                limit_token = statement.tokens[pos + 2]
+                break
+        if not limit_token:
+            return limit_token
+        return self._get_limit_from_token(limit_token)
+
+    def get_substring_before_and_after_limit(self):
+        """This function returns a tuple containing """
+        """the substring before and after the limit"""
+        before_str = ''
+        after_str = ''
+        limit_pos = None
+
+        # Add all items to before_str until there is a limit
+        for pos, item in enumerate(self._parsed[0].tokens):
 
 Review comment:
   Given you're now using `sqlparse` rather than trying to extract the before and after portions of the query, why don't you simply replace the relevant token in-place, i.e., 
   ```
   >>> sqlparse.parse('SELECT * FROM foo LIMIT 1000')[0].tokens
   [..., <Keyword 'LIMIT' at 0x10DE741F0>, <Whitespace ' ' at 0x10DE74258>, <Integer '1000' at 0x10DE742C0>]
   ```
   and
   ```
   >>> sqlparse.parse('SELECT * FROM foo LIMIT 10, 1000')[0].tokens
   [..., <Keyword 'LIMIT' at 0x10DE74668>, <Whitespace ' ' at 0x10DE746D0>, <IdentifierList '10, 10...' at 0x10DE751D0>]
   ```
   It seems once you find the `LIMIT` keyword just jump two tokens which will contain either an `IdentifierList` or `Integer` and update the token accordingly, i.e., for the first case (example code):
   ```
   >>> s = sqlparse.parse('SELECT * FROM foo LIMIT 1000')[0]
   >>> s.tokens[-1].value = '999'
   >>> str(s)
   'SELECT * FROM foo LIMIT 999'
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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