Skip to content

Commit

Permalink
replace max global limit with existing sql max row limit
Browse files Browse the repository at this point in the history
  • Loading branch information
villebro committed Sep 15, 2021
1 parent 8181d7b commit 769bf3e
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 21 deletions.
2 changes: 1 addition & 1 deletion superset/common/query_object.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ def __init__( # pylint: disable=too-many-arguments,too-many-locals
else config["ROW_LIMIT"]
)
self.row_limit = apply_max_row_limit(
config["MAX_GLOBAL_ROW_LIMIT"], row_limit or default_row_limit,
config["SQL_MAX_ROW"], row_limit or default_row_limit,
)
self.row_offset = row_offset or 0
self.filter = filters or []
Expand Down
10 changes: 3 additions & 7 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
# default viz used in chart explorer
DEFAULT_VIZ_TYPE = "table"

# maximum row limit that can be set on a query. Will override the row limit defined
# in all queries (both SQL Lab and chart data queries)
MAX_GLOBAL_ROW_LIMIT: Optional[int] = None
# default row limit to apply to queries unless set in the query object
# default row limit to apply to chart data requests unless if unset
ROW_LIMIT = 50000
# default row limit when requesting samples from datasource in explore view
SAMPLES_ROW_LIMIT = 1000
Expand Down Expand Up @@ -673,9 +670,8 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]:
# Set this API key to enable Mapbox visualizations
MAPBOX_API_KEY = os.environ.get("MAPBOX_API_KEY", "")

# Maximum number of rows returned from a database
# in async mode, no more than SQL_MAX_ROW will be returned and stored
# in the results backend. This also becomes the limit when exporting CSVs
# Maximum number of rows returned from a database, both SQL Lab, chart data
# requests and CSV exports
SQL_MAX_ROW = 100000

# Maximum number of rows displayed in SQL Lab UI
Expand Down
5 changes: 3 additions & 2 deletions superset/utils/sqllab_execution_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,9 @@ def _get_template_params(query_params: Dict[str, Any]) -> Dict[str, Any]:

@staticmethod
def _get_limit_param(query_params: Dict[str, Any]) -> int:
limit: int = query_params.get("queryLimit") or app.config["SQL_MAX_ROW"]
limit = apply_max_row_limit(app.config["MAX_GLOBAL_ROW_LIMIT"], limit)
limit = apply_max_row_limit(
app.config["SQL_MAX_ROW"], query_params.get("queryLimit", 0)
)
if limit < 0:
logger.warning(
"Invalid limit of %i specified. Defaulting to max limit.", limit
Expand Down
2 changes: 1 addition & 1 deletion superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,7 @@ def filter( # pylint: disable=no-self-use

datasource.raise_for_access()
row_limit = apply_max_row_limit(
config["MAX_GLOBAL_ROW_LIMIT"], config["FILTER_SELECT_ROW_LIMIT"]
config["SQL_MAX_ROW"], config["FILTER_SELECT_ROW_LIMIT"]
)
payload = json.dumps(
datasource.values_for_column(column, row_limit),
Expand Down
2 changes: 1 addition & 1 deletion superset/viz.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ def query_obj(self) -> QueryObjectDict:

# apply row limit to query
row_limit = form_data.get("row_limit") or config["ROW_LIMIT"]
row_limit = apply_max_row_limit(config["MAX_GLOBAL_ROW_LIMIT"], row_limit)
row_limit = apply_max_row_limit(config["SQL_MAX_ROW"], row_limit)

# default order direction
order_desc = form_data.get("order_desc", True)
Expand Down
16 changes: 7 additions & 9 deletions tests/integration_tests/charts/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1211,10 +1211,9 @@ 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, "MAX_GLOBAL_ROW_LIMIT": 10},
"superset.common.query_object.config", {**app.config, "SQL_MAX_ROW": 10},
)
def test_chart_data_max_global_row_limit(self):
def test_chart_data_sql_max_row_limit(self):
"""
Chart data API: Ensure row count doesn't exceed max global row limit
"""
Expand Down Expand Up @@ -1246,12 +1245,12 @@ def test_chart_data_sample_default_limit(self):
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
@mock.patch(
"superset.common.query_actions.config",
{**app.config, "SAMPLES_ROW_LIMIT": 5, "MAX_GLOBAL_ROW_LIMIT": 15},
{**app.config, "SAMPLES_ROW_LIMIT": 5, "SQL_MAX_ROW": 15},
)
def test_chart_data_sample_custom_limit(self):
"""
Chart data API: Ensure requested sample response row count is between
default and max global limit
default and SQL max row limit
"""
self.login(username="admin")
request_payload = get_query_context("birth_names")
Expand All @@ -1264,13 +1263,12 @@ 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, "MAX_GLOBAL_ROW_LIMIT": 5},
"superset.common.query_object.config", {**app.config, "SQL_MAX_ROW": 5},
)
def test_chart_data_max_global_sample_limit(self):
def test_chart_data_sql_max_row_sample_limit(self):
"""
Chart data API: Ensure requested sample response row count doesn't
exceed max global row limit
exceed SQL max row limit
"""
self.login(username="admin")
request_payload = get_query_context("birth_names")
Expand Down

0 comments on commit 769bf3e

Please sign in to comment.