[SQL Lab] Clarify SQL Lab query and display limits (#7641)

superset/assets/src/SqlLab/components/SqlEditor.jsx

@@ -346,7 +346,11 @@ class SqlEditor extends React.PureComponent {
     }
     const qe = this.props.queryEditor;
     let limitWarning = null;
-    if (this.props.latestQuery && this.props.latestQuery.limit_reached) {
+    if (
+      this.props.latestQuery
+      && this.props.latestQuery.results
+      && this.props.latestQuery.results.displayLimitReached
+    ) {
       const tooltip = (
         <Tooltip id="tooltip">
           {t(`It appears that the number of rows in the query results displayed

superset/config.py

@@ -351,7 +351,13 @@ MAPBOX_API_KEY = os.environ.get('MAPBOX_API_KEY', '')
 # in the results backend. This also becomes the limit when exporting CSVs
 SQL_MAX_ROW = 100000
 
-# Default row limit for SQL Lab queries
+# Maximum number of rows displayed in SQL Lab UI
+# Is set to avoid out of memory/localstorage issues in browsers. Does not affect
+# exported CSVs
+DISPLAY_MAX_ROW = 10000
+
+# Default row limit for SQL Lab queries. Is overridden by setting a new limit in
+# the SQL Lab UI
 DEFAULT_SQLLAB_LIMIT = 1000
 
 # Maximum number of tables/views displayed in the dropdown window in SQL Lab.

superset/migrations/versions/d7c1a0d6f2da_remove_limit_used_from_query_model.py

@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Remove limit used from query model
+
+Revision ID: d7c1a0d6f2da
+Revises: afc69274c25a
+Create Date: 2019-06-04 10:12:36.675369
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'd7c1a0d6f2da'
+down_revision = 'afc69274c25a'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    with op.batch_alter_table('query') as batch_op:
+        batch_op.drop_column('limit_used')
+
+
+def downgrade():
+    op.add_column('query', sa.Column('limit_used', sa.BOOLEAN(), nullable=True))

superset/models/sql_lab.py

@@ -59,7 +59,6 @@ class Query(Model, ExtraJSONMixin):
     executed_sql = Column(Text)
     # Could be configured in the superset config.
     limit = Column(Integer)
-    limit_used = Column(Boolean, default=False)
     select_as_cta = Column(Boolean)
     select_as_cta_used = Column(Boolean, default=False)
 
@@ -94,10 +93,6 @@ class Query(Model, ExtraJSONMixin):
         sqla.Index('ti_user_id_changed_on', user_id, changed_on),
     )
 
-    @property
-    def limit_reached(self):
-        return self.rows == self.limit if self.limit_used else False
-
     def to_dict(self):
         return {
             'changedOn': self.changed_on,
@@ -122,7 +117,6 @@ class Query(Model, ExtraJSONMixin):
             'tempTable': self.tmp_table_name,
             'userId': self.user_id,
             'user': user_label(self.user),
-            'limit_reached': self.limit_reached,
             'resultsKey': self.results_key,
             'trackingUrl': self.tracking_url,
             'extra': self.extra,

superset/views/core.py

@@ -67,7 +67,9 @@ from .base import (
     get_error_msg, handle_api_exception, json_error_response, json_success,
     SupersetFilter, SupersetModelView, YamlExportMixin,
 )
-from .utils import bootstrap_user_data, get_datasource_info, get_form_data, get_viz
+from .utils import (
+    apply_display_max_row_limit, bootstrap_user_data, get_datasource_info, get_form_data,
+    get_viz)
 
 config = app.config
 CACHE_DEFAULT_TIMEOUT = config.get('CACHE_DEFAULT_TIMEOUT', 0)
@@ -2513,13 +2515,11 @@ class Superset(BaseSupersetView):
                 '{}'.format(rejected_tables)), status=403)
 
         payload = utils.zlib_decompress_to_string(blob)
-        display_limit = app.config.get('DEFAULT_SQLLAB_LIMIT', None)
-        if display_limit:
-            payload_json = json.loads(payload)
-            payload_json['data'] = payload_json['data'][:display_limit]
+        payload_json = json.loads(payload)
+
         return json_success(
             json.dumps(
-                payload_json,
+                apply_display_max_row_limit(payload_json),
                 default=utils.json_iso_dttm_ser,
                 ignore_nan=True,
             ),
@@ -2725,8 +2725,9 @@ class Superset(BaseSupersetView):
                     rendered_query,
                     return_results=True,
                     user_name=g.user.username if g.user else None)
+
             payload = json.dumps(
-                data,
+                apply_display_max_row_limit(data),
                 default=utils.pessimistic_json_iso_dttm_ser,
                 ignore_nan=True,
                 encoding=None,

superset/views/utils.py

@@ -16,6 +16,7 @@
 # under the License.
 # pylint: disable=C,R,W
 from collections import defaultdict
+from typing import Any, Dict, List
 from urllib import parse
 
 from flask import g, request
@@ -26,9 +27,10 @@ from superset import app, db, viz
 from superset.connectors.connector_registry import ConnectorRegistry
 from superset.legacy import update_time_range
 import superset.models.core as models
+from superset.utils.core import QueryStatus
 
 
-FORM_DATA_KEY_BLACKLIST = []
+FORM_DATA_KEY_BLACKLIST: List[str] = []
 if not app.config.get('ENABLE_JAVASCRIPT_CONTROLS'):
     FORM_DATA_KEY_BLACKLIST = [
         'js_tooltip',
@@ -184,3 +186,26 @@ def get_datasource_info(datasource_id, datasource_type, form_data):
             'The datasource associated with this chart no longer exists')
     datasource_id = int(datasource_id)
     return datasource_id, datasource_type
+
+
+def apply_display_max_row_limit(sql_results: Dict[str, Any]) -> Dict[str, Any]:
+    """
+    Given a `sql_results` nested structure, applies a limit to the number of rows
+
+    `sql_results` here is the nested structure coming out of sql_lab.get_sql_results, it
+    contains metadata about the query, as well as the data set returned by the query.
+    This method limits the number of rows adds a `displayLimitReached: True` flag to the
+    metadata.
+
+    :param sql_results: The results of a sql query from sql_lab.get_sql_results
+    :returns: The mutated sql_results structure
+    """
+    display_limit = app.config.get('DISPLAY_MAX_ROW')
+    if (
+        display_limit and
+        sql_results['status'] == QueryStatus.SUCCESS and
+        display_limit < sql_results['query']['rows']
+    ):
+        sql_results['data'] = sql_results['data'][:display_limit]
+        sql_results['displayLimitReached'] = True
+    return sql_results

tests/celery_tests.py

@@ -205,7 +205,6 @@ class CeleryTestCase(SupersetTestCase):
             'LIMIT 666', query.executed_sql)
         self.assertEqual(sql_where, query.sql)
         self.assertEqual(0, query.rows)
-        self.assertEqual(False, query.limit_used)
         self.assertEqual(True, query.select_as_cta)
         self.assertEqual(True, query.select_as_cta_used)