make max_limit optional in util

superset/common/query_object.py

@@ -191,9 +191,7 @@ def __init__(  # pylint: disable=too-many-arguments,too-many-locals
             if self.result_type == ChartDataResultType.SAMPLES
             else config["ROW_LIMIT"]
         )
-        self.row_limit = apply_max_row_limit(
-            config["SQL_MAX_ROW"], row_limit or default_row_limit,
-        )
+        self.row_limit = apply_max_row_limit(row_limit or default_row_limit)
         self.row_offset = row_offset or 0
         self.filter = filters or []
         self.timeseries_limit = timeseries_limit

superset/utils/core.py

@@ -1764,27 +1764,23 @@ def parse_boolean_string(bool_str: Optional[str]) -> bool:
         return False
 
 
-def apply_max_row_limit(max_limit: Optional[int], limit: int) -> int:
+def apply_max_row_limit(limit: int, max_limit: Optional[int] = None,) -> int:
     """
     Override row limit if max global limit is defined
 
-    :param max_limit: Maximum allowed row limit
     :param limit: requested row limit
+    :param max_limit: Maximum allowed row limit
     :return: Capped row limit
 
-    >>> apply_max_row_limit(None, 10000)
-    10000
     >>> apply_max_row_limit(100000, 10)
     10
     >>> apply_max_row_limit(10, 100000)
     10
-    >>> apply_max_row_limit(10, 0)
-    10
-    >>> apply_max_row_limit(0, 0)
-    0
-    """
-    if max_limit is not None and max_limit != 0:
-        if limit != 0:
-            return min(max_limit, limit)
-        return max_limit
-    return limit
+    >>> apply_max_row_limit(0, 10000)
+    10000
+    """
+    if max_limit is None:
+        max_limit = current_app.config["SQL_MAX_ROW"]
+    if limit != 0:
+        return min(max_limit, limit)
+    return max_limit

superset/utils/sqllab_execution_context.py

@@ -23,7 +23,7 @@
 
 from flask import g
 
-from superset import app, is_feature_enabled
+from superset import is_feature_enabled
 from superset.models.sql_lab import Query
 from superset.sql_parse import CtasMethod
 from superset.utils import core as utils
@@ -103,9 +103,7 @@ def _get_template_params(query_params: Dict[str, Any]) -> Dict[str, Any]:
 
     @staticmethod
     def _get_limit_param(query_params: Dict[str, Any]) -> int:
-        limit = apply_max_row_limit(
-            app.config["SQL_MAX_ROW"], query_params.get("queryLimit") or 0
-        )
+        limit = apply_max_row_limit(query_params.get("queryLimit") or 0)
         if limit < 0:
             logger.warning(
                 "Invalid limit of %i specified. Defaulting to max limit.", limit

superset/views/core.py

@@ -897,9 +897,7 @@ def filter(  # pylint: disable=no-self-use
             return json_error_response(DATASOURCE_MISSING_ERR)
 
         datasource.raise_for_access()
-        row_limit = apply_max_row_limit(
-            config["SQL_MAX_ROW"], config["FILTER_SELECT_ROW_LIMIT"]
-        )
+        row_limit = apply_max_row_limit(config["FILTER_SELECT_ROW_LIMIT"])
         payload = json.dumps(
             datasource.values_for_column(column, row_limit),
             default=utils.json_int_dttm_ser,

superset/viz.py

@@ -328,7 +328,7 @@ def query_obj(self) -> QueryObjectDict:  # pylint: disable=too-many-locals
 
         # apply row limit to query
         row_limit = int(self.form_data.get("row_limit") or config["ROW_LIMIT"])
-        row_limit = apply_max_row_limit(config["SQL_MAX_ROW"], row_limit)
+        row_limit = apply_max_row_limit(row_limit)
 
         # default order direction
         order_desc = self.form_data.get("order_desc", True)

tests/integration_tests/charts/api_tests.py

@@ -18,7 +18,7 @@
 """Unit tests for Superset"""
 import json
 import unittest
-from datetime import datetime, timedelta
+from datetime import datetime
 from io import BytesIO
 from typing import Optional
 from unittest import mock
@@ -35,7 +35,7 @@
 import prison
 import pytest
 import yaml
-from sqlalchemy import and_, or_
+from sqlalchemy import and_
 from sqlalchemy.sql import func
 
 from tests.integration_tests.fixtures.world_bank_dashboard import (
@@ -1211,7 +1211,7 @@ def test_chart_data_default_row_limit(self):
 
     @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
     @mock.patch(
-        "superset.common.query_object.config", {**app.config, "SQL_MAX_ROW": 10},
+        "superset.utils.core.current_app.config", {**app.config, "SQL_MAX_ROW": 10},
     )
     def test_chart_data_sql_max_row_limit(self):
         """
@@ -1263,7 +1263,7 @@ def test_chart_data_sample_custom_limit(self):
 
     @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
     @mock.patch(
-        "superset.common.query_object.config", {**app.config, "SQL_MAX_ROW": 5},
+        "superset.utils.core.current_app.config", {**app.config, "SQL_MAX_ROW": 5},
     )
     def test_chart_data_sql_max_row_sample_limit(self):
         """

tests/integration_tests/charts/schema_tests.py

@@ -18,17 +18,23 @@
 """Unit tests for Superset"""
 from unittest import mock
 
+import pytest
+
 from marshmallow import ValidationError
 from tests.integration_tests.test_app import app
 from superset.charts.schemas import ChartDataQueryContextSchema
 from tests.integration_tests.base_tests import SupersetTestCase
+from tests.integration_tests.fixtures.birth_names_dashboard import (
+    load_birth_names_dashboard_with_slices,
+)
 from tests.integration_tests.fixtures.query_context import get_query_context
 
 
 class TestSchema(SupersetTestCase):
     @mock.patch(
-        "superset.common.query_object.config", {**app.config, "SQL_MAX_ROW": 100000},
+        "superset.common.query_object.config", {**app.config, "ROW_LIMIT": 5000},
     )
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
     def test_query_context_limit_and_offset(self):
         self.login(username="admin")
         payload = get_query_context("birth_names")
@@ -38,7 +44,7 @@ def test_query_context_limit_and_offset(self):
         payload["queries"][0].pop("row_offset", None)
         query_context = ChartDataQueryContextSchema().load(payload)
         query_object = query_context.queries[0]
-        self.assertEqual(query_object.row_limit, app.config["ROW_LIMIT"])
+        self.assertEqual(query_object.row_limit, 5000)
         self.assertEqual(query_object.row_offset, 0)
 
         # Valid limit and offset
@@ -57,12 +63,14 @@ def test_query_context_limit_and_offset(self):
         self.assertIn("row_limit", context.exception.messages["queries"][0])
         self.assertIn("row_offset", context.exception.messages["queries"][0])
 
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
     def test_query_context_null_timegrain(self):
         self.login(username="admin")
         payload = get_query_context("birth_names")
         payload["queries"][0]["extras"]["time_grain_sqla"] = None
         _ = ChartDataQueryContextSchema().load(payload)
 
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
     def test_query_context_series_limit(self):
         self.login(username="admin")
         payload = get_query_context("birth_names")
@@ -84,6 +92,7 @@ def test_query_context_series_limit(self):
         }
         _ = ChartDataQueryContextSchema().load(payload)
 
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
     def test_query_context_null_post_processing_op(self):
         self.login(username="admin")
         payload = get_query_context("birth_names")