Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 65 additions & 14 deletions superset/models/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
_(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A secondary temporal filter like "2024-01-01T00:00:00Z : 2024-01-02" still returns a 500: get_since_until compares an offset-aware _since with a naive _until (_since > _until) and raises TypeError, which isn't in this tuple, so it escapes the new handler. The same gap applies to the other guards added here. Should TypeError be caught too, or is mixed-timezone input intentionally out of scope for this fix?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In commit 089c8d37a4, we addressed this by adding TypeError to the caught exception tuple across all three get_since_until_from_time_range call sites in superset/models/helpers.py (lines 3089, 3227, and 7027). When mixed offset-naive and offset-aware datetimes like "2024-01-01T00:00:00Z : 2024-01-02" are passed, get_since_until raises a TypeError during timestamp comparison (_since > _until). This is now properly intercepted and converted into a QueryObjectValidationError, consistently returning 400 instead of 500.

Additionally, we formatted the exception statement in superset/models/helpers.py to satisfy ruff-format (resolving the pre-commit (current) CI failure) and added a dedicated regression unit test in tests/unit_tests/models/helpers_test.py verifying mixed-timezone temporal ranges.

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,
Expand Down
243 changes: 243 additions & 0 deletions tests/unit_tests/models/helpers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Loading