diff --git a/superset/common/query_object.py b/superset/common/query_object.py index 72995c0668059..1a843cf71d6a5 100644 --- a/superset/common/query_object.py +++ b/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 diff --git a/superset/utils/core.py b/superset/utils/core.py index 538e99fb9e80d..7146a3b57e1ae 100644 --- a/superset/utils/core.py +++ b/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 diff --git a/superset/utils/sqllab_execution_context.py b/superset/utils/sqllab_execution_context.py index 58d7f3104fa80..09ae33d54da51 100644 --- a/superset/utils/sqllab_execution_context.py +++ b/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 diff --git a/superset/views/core.py b/superset/views/core.py index 561cc701cd183..a07f7d509bdf5 100755 --- a/superset/views/core.py +++ b/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, diff --git a/superset/viz.py b/superset/viz.py index afa3d7180d42b..3ab2e2b564fd0 100644 --- a/superset/viz.py +++ b/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) diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index 18aba60303e46..b9b8f30bb92f0 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/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): """ diff --git a/tests/integration_tests/charts/schema_tests.py b/tests/integration_tests/charts/schema_tests.py index 5cdf22ac338e1..977cf72957396 100644 --- a/tests/integration_tests/charts/schema_tests.py +++ b/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")