betodealmeida commented on a change in pull request #13521:
URL: https://github.com/apache/superset/pull/13521#discussion_r619533886
##########
File path: superset/sql_lab.py
##########
@@ -190,6 +190,10 @@ def execute_sql_statement(
db_engine_spec = database.db_engine_spec
parsed_query = ParsedQuery(sql_statement)
sql = parsed_query.stripped()
+ # This is a test to see if the query is being
+ # limited by either the dropdown or the sql.
+ # We are testing to see if more rows exist than the limit.
+ increased_query = None if query.limit is None else query.limit + 1
Review comment:
Can you rename this to `increased_limit`? The current name suggests that
this is a query, but it's actually a limit.
##########
File path: superset/sql_lab.py
##########
@@ -245,7 +252,20 @@ def execute_sql_statement(
query.id,
str(query.to_dict()),
)
- data = db_engine_spec.fetch_data(cursor, query.limit)
+ data = db_engine_spec.fetch_data(cursor, increased_query)
+ if query.limit is None or len(data) <= query.limit:
+ query.limiting_factor = LimitingFactor.NOT_LIMITED
+ else:
+ # returning 1 row less then the query.limit
Review comment:
We're actually returning 1 less than `increased_query` here:
```suggestion
# return 1 row less than increased_query
```
##########
File path: superset/views/core.py
##########
@@ -2384,6 +2384,7 @@ def _sql_json_sync(
# Update saved query if needed
QueryDAO.update_saved_query_exec_info(query_id)
+ # TODO: set LimitingFactor to display?
Review comment:
Let's get rid of this.
##########
File path: superset/sql_parse.py
##########
@@ -332,8 +332,9 @@ def set_or_update_query_limit(self, new_limit: int) -> str:
break
_, limit = statement.token_next(idx=limit_pos)
# Override the limit only when it exceeds the configured value.
- if limit.ttype == sqlparse.tokens.Literal.Number.Integer and new_limit
< int(
- limit.value
+ if force or (
+ limit.ttype == sqlparse.tokens.Literal.Number.Integer
+ and (new_limit < int(limit.value))
Review comment:
We actually need to have this condition like we did the first time:
```suggestion
if (
limit.ttype == sqlparse.tokens.Literal.Number.Integer
and (force or new_limit < int(limit.value))
```
Otherwise we'll change the next block (`elif limit.is_group:`), which TBH I
don't understand.
##########
File path: superset/sql_lab.py
##########
@@ -245,7 +252,20 @@ def execute_sql_statement(
query.id,
str(query.to_dict()),
)
- data = db_engine_spec.fetch_data(cursor, query.limit)
+ data = db_engine_spec.fetch_data(cursor, increased_query)
+ if query.limit is None or len(data) <= query.limit:
+ query.limiting_factor = LimitingFactor.NOT_LIMITED
+ else:
+ # returning 1 row less then the query.limit
+ data = data[:-1]
+
+ except SoftTimeLimitExceeded as ex:
Review comment:
Did you add this or is it from a borked merge?
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
