diff --git a/superset/models/helpers.py b/superset/models/helpers.py index cd8c27edee0e..61884ed6c008 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -61,6 +61,7 @@ from flask_babel import get_locale, lazy_gettext as _ from jinja2.exceptions import TemplateError, UndefinedError from markupsafe import escape, Markup +from marshmallow import ValidationError from pandas import DateOffset, Timedelta from sqlalchemy import and_, Column, or_, UniqueConstraint from sqlalchemy.exc import MultipleResultsFound @@ -88,6 +89,10 @@ from superset import db, is_feature_enabled from superset.advanced_data_type.types import AdvancedDataTypeResponse +from superset.commands.chart.exceptions import ( + TimeRangeAmbiguousError, + TimeRangeParseFailError, +) from superset.common.db_query_status import QueryStatus from superset.common.grouping_sets import ( grouping_marker_label, @@ -3037,7 +3042,18 @@ def processing_time_offsets( # pylint: disable=too-many-locals,too-many-stateme cache_keys: list[str | None] = [] offset_dfs: dict[str, pd.DataFrame] = {} - outer_from_dttm, outer_to_dttm = get_since_until_from_query_object(query_object) + try: + outer_from_dttm, outer_to_dttm = get_since_until_from_query_object( + query_object + ) + except ( + TimeRangeParseFailError, + TimeRangeAmbiguousError, + ValidationError, + ValueError, + ) as ex: + raise QueryObjectValidationError(str(ex)) from ex + if not outer_from_dttm or not outer_to_dttm: raise QueryObjectValidationError( _( @@ -3065,7 +3081,13 @@ def processing_time_offsets( # pylint: disable=too-many-locals,too-many-stateme offset_from_dttm, offset_to_dttm = ( get_since_until_from_time_range(time_range=offset) ) - except ValueError as ex: + except ( + TimeRangeParseFailError, + TimeRangeAmbiguousError, + ValidationError, + ValueError, + TypeError, + ) as ex: raise QueryObjectValidationError(str(ex)) from ex # Use the specified range directly @@ -3189,13 +3211,26 @@ def processing_time_offsets( # pylint: disable=too-many-locals,too-many-stateme flt.get("val"), str ): time_range = cast(str, flt.get("val")) - ( - new_outer_from_dttm, - new_outer_to_dttm, - ) = get_since_until_from_time_range( - time_range=time_range, - time_shift=offset, - ) + try: + ( + new_outer_from_dttm, + new_outer_to_dttm, + ) = get_since_until_from_time_range( + time_range=time_range, + time_shift=offset, + ) + except ( + TimeRangeParseFailError, + TimeRangeAmbiguousError, + ValidationError, + ValueError, + TypeError, + ) as ex: + col_name = flt.get("col") or "unknown" + raise QueryObjectValidationError( + f"Cannot parse time range for filter " + f"'{col_name}': {ex}" + ) from ex flt["val"] = f"{new_outer_from_dttm} : {new_outer_to_dttm}" else: # If it IS a datetime series, we still need to clear conflicts @@ -6980,11 +7015,27 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma and isinstance(eq, str) and col_obj is not None ): - _since, _until = get_since_until_from_time_range( - time_range=eq, - time_shift=time_shift, - extras=extras, - ) + try: + _since, _until = get_since_until_from_time_range( + time_range=eq, + time_shift=time_shift, + extras=extras, + ) + except ( + TimeRangeParseFailError, + TimeRangeAmbiguousError, + ValidationError, + ValueError, + TypeError, + ) as ex: + col_name = ( + col_obj.column_name + if hasattr(col_obj, "column_name") + else str(col_obj) + ) + raise QueryObjectValidationError( + f"Cannot parse time range for filter '{col_name}': {ex}" + ) from ex _temporal_filter = self.get_time_filter( time_col=col_obj, start_dttm=_since, diff --git a/tests/unit_tests/models/helpers_test.py b/tests/unit_tests/models/helpers_test.py index 7a2f39fceacd..6b07bf54a9de 100644 --- a/tests/unit_tests/models/helpers_test.py +++ b/tests/unit_tests/models/helpers_test.py @@ -6821,3 +6821,246 @@ def compile_not_in() -> str: kql = compile_not_in() assert "isnull(state)" in kql assert "IS NULL" not in kql + + +def test_get_sqla_query_malformed_temporal_range_raises_validation_error( + database: Database, +) -> None: + """ + Test that an invalid temporal range filter (e.g. 'a : b' or inverted dates) + raises QueryObjectValidationError naming the column, returning 400 instead of 500. + Fixes #45459. + """ + from superset.commands.chart.exceptions import TimeRangeParseFailError + from superset.connectors.sqla.models import SqlaTable, TableColumn + from superset.exceptions import QueryObjectValidationError + + table = SqlaTable( + database=database, + schema=None, + table_name="t", + columns=[ + TableColumn(column_name="ds", type="TIMESTAMP", is_dttm=True), + TableColumn(column_name="other_ds", type="TIMESTAMP", is_dttm=True), + ], + ) + + with pytest.raises(QueryObjectValidationError, match="other_ds") as exc_info: + table.get_sqla_query( + columns=["ds"], + metrics=[], + extras={}, + filter=[ + { + "col": "ds", + "op": FilterOperator.TEMPORAL_RANGE, + "val": "2024-01-01 : 2024-01-02", + }, + { + "col": "other_ds", + "op": FilterOperator.TEMPORAL_RANGE, + "val": "a : b", + }, + ], + granularity=None, + is_timeseries=False, + ) + assert isinstance(exc_info.value.__cause__, (TimeRangeParseFailError, ValueError)) + + with pytest.raises(QueryObjectValidationError, match="other_ds") as exc_info: + table.get_sqla_query( + columns=["ds"], + metrics=[], + extras={}, + filter=[ + { + "col": "ds", + "op": FilterOperator.TEMPORAL_RANGE, + "val": "2024-01-01 : 2024-01-02", + }, + { + "col": "other_ds", + "op": FilterOperator.TEMPORAL_RANGE, + "val": "2024-02-01 : 2024-01-01", + }, + ], + granularity=None, + is_timeseries=False, + ) + assert isinstance(exc_info.value.__cause__, (TimeRangeParseFailError, ValueError)) + + with pytest.raises(QueryObjectValidationError, match="other_ds") as exc_info: + table.get_sqla_query( + columns=["ds"], + metrics=[], + extras={}, + filter=[ + { + "col": "ds", + "op": FilterOperator.TEMPORAL_RANGE, + "val": "2024-01-01 : 2024-01-02", + }, + { + "col": "other_ds", + "op": FilterOperator.TEMPORAL_RANGE, + "val": "2024-01-01T00:00:00Z : 2024-01-02", + }, + ], + granularity=None, + is_timeseries=False, + ) + assert isinstance( + exc_info.value.__cause__, + (TimeRangeParseFailError, ValueError, TypeError), + ) + + +def test_processing_time_offsets_malformed_relative_filter_raises_validation_error( + database: Database, +) -> None: + """ + Test that processing_time_offsets raises QueryObjectValidationError naming + the column and chaining TimeRangeParseFailError when rewriting temporal filters + for relative time shifts. + Fixes #45459. + """ + import pandas as pd + + from superset.commands.chart.exceptions import TimeRangeParseFailError + from superset.common.query_object import QueryObject + from superset.connectors.sqla.models import SqlaTable, TableColumn + from superset.exceptions import QueryObjectValidationError + + table = SqlaTable( + database=database, + schema=None, + table_name="t", + columns=[ + TableColumn(column_name="category", type="VARCHAR(255)"), + TableColumn(column_name="ds", type="TIMESTAMP", is_dttm=True), + TableColumn(column_name="other_ds", type="TIMESTAMP", is_dttm=True), + ], + ) + + query_object = QueryObject( + columns=["category"], + time_range="2024-01-01 : 2024-01-02", + filters=[ + { + "col": "ds", + "op": FilterOperator.TEMPORAL_RANGE, + "val": "2024-01-01 : 2024-01-02", + }, + { + "col": "other_ds", + "op": FilterOperator.TEMPORAL_RANGE, + "val": "a : b", + }, + ], + time_offsets=["1 day ago"], + ) + + df = pd.DataFrame({"category": ["A", "B"]}) + + with pytest.raises(QueryObjectValidationError, match="other_ds") as exc_info: + table.processing_time_offsets(df, query_object) + + assert isinstance(exc_info.value.__cause__, TimeRangeParseFailError) + + +def test_processing_time_offsets_malformed_date_range_offset_raises_validation_error( + database: Database, + mocker: MockerFixture, +) -> None: + """ + Test that processing_time_offsets raises QueryObjectValidationError when + DATE_RANGE_TIMESHIFTS_ENABLED is enabled and a date range offset is invalid. + Fixes #45459. + """ + import pandas as pd + + from superset.commands.chart.exceptions import TimeRangeParseFailError + from superset.common.query_object import QueryObject + from superset.connectors.sqla.models import SqlaTable, TableColumn + from superset.exceptions import QueryObjectValidationError + + mocker.patch( + "superset.models.helpers.feature_flag_manager.is_feature_enabled", + side_effect=lambda feature: feature == "DATE_RANGE_TIMESHIFTS_ENABLED", + ) + + table = SqlaTable( + database=database, + schema=None, + table_name="t", + columns=[ + TableColumn(column_name="ds", type="TIMESTAMP", is_dttm=True), + ], + ) + + query_object = QueryObject( + columns=["ds"], + time_range="2024-01-01 : 2024-01-02", + filters=[ + { + "col": "ds", + "op": FilterOperator.TEMPORAL_RANGE, + "val": "2024-01-01 : 2024-01-02", + }, + ], + time_offsets=["2024-02-01 : 2024-01-01"], + ) + + df = pd.DataFrame({"ds": [pd.Timestamp("2024-01-01")]}) + + with pytest.raises(QueryObjectValidationError) as exc_info: + table.processing_time_offsets(df, query_object) + + assert isinstance( + exc_info.value.__cause__, + (TimeRangeParseFailError, ValueError), + ) + + +def test_processing_time_offsets_malformed_outer_time_range_raises_validation_error( + database: Database, +) -> None: + """ + Test that processing_time_offsets raises QueryObjectValidationError when + the outer time range itself is malformed. + Fixes #45459. + """ + import pandas as pd + + from superset.commands.chart.exceptions import TimeRangeParseFailError + from superset.common.query_object import QueryObject + from superset.connectors.sqla.models import SqlaTable, TableColumn + from superset.exceptions import QueryObjectValidationError + + table = SqlaTable( + database=database, + schema=None, + table_name="t", + columns=[ + TableColumn(column_name="ds", type="TIMESTAMP", is_dttm=True), + ], + ) + + query_object = QueryObject( + columns=["ds"], + time_range="a : b", + filters=[ + {"col": "ds", "op": FilterOperator.TEMPORAL_RANGE, "val": "a : b"}, + ], + time_offsets=["1 day ago"], + ) + + df = pd.DataFrame({"ds": [pd.Timestamp("2024-01-01")]}) + + with pytest.raises(QueryObjectValidationError) as exc_info: + table.processing_time_offsets(df, query_object) + + assert isinstance( + exc_info.value.__cause__, + (TimeRangeParseFailError, ValueError), + )