betodealmeida commented on a change in pull request #13521:
URL: https://github.com/apache/superset/pull/13521#discussion_r616926336



##########
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()
+    if query.limit is None:
+        limited_query = query.limit
+    else:
+        limited_query = query.limit + 1

Review comment:
       The name `limited_query` is a bit confusing because it suggests that 
this is a query. Maybe use `increased_limit`?
   
   ```suggestion
        increased_limit = None if query.limit is None else query.limit + 1
   ```

##########
File path: superset/sql_lab.py
##########
@@ -245,7 +252,22 @@ def execute_sql_statement(
                 query.id,
                 str(query.to_dict()),
             )
-            data = db_engine_spec.fetch_data(cursor, query.limit)
+            # 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.
+            data = db_engine_spec.fetch_data(cursor, limited_query)
+            if query.limit is not None and len(data) <= query.limit:

Review comment:
       I think this needs to be:
   
   ```suggestion
               if query.limit is None or len(data) <= query.limit:
   ```
   
   Since if `query.limit is None` the query was also not limited.

##########
File path: superset/sql_lab.py
##########
@@ -215,11 +219,14 @@ def execute_sql_statement(
         if SQL_MAX_ROW and (not query.limit or query.limit > SQL_MAX_ROW):
             query.limit = SQL_MAX_ROW
         if query.limit:
-            sql = database.apply_limit_to_sql(sql, query.limit)
+            # We are fetching one more than the requested limit in order
+            # to test whether there are more rows than the limit.
+            # Later, the extra row will be dropped before sending
+            # the results back to the user.

Review comment:
       We need to move this comment above where we define `increased_query`.




-- 
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]

Reply via email to