diff --git a/superset/models/helpers.py b/superset/models/helpers.py index 2f3208c8208..a336dfaf56c 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -1473,7 +1473,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods # Determine the temporal column with multiple fallback strategies temporal_col = self._get_temporal_column_for_filter( - query_object_clone, x_axis_label + query_object, x_axis_label ) # Always add a temporal filter for date range offsets @@ -1698,31 +1698,32 @@ class ExploreMixin: # pylint: disable=too-many-public-methods Helper method to reliably determine the temporal column for filtering. This method tries multiple strategies to find the correct temporal column: - 1. Use explicitly set granularity - 2. Use x_axis_label if it's a temporal column - 3. Find any datetime column in the datasource + 1. Use the column from existing TEMPORAL_RANGE filter + 2. Use explicitly set granularity + 3. Use x_axis_label if it exists :param query_object: The query object :param x_axis_label: The x-axis label from the query :return: The name of the temporal column, or None if not found """ - # Strategy 1: Use explicitly set granularity + # Strategy 1: Use the column from existing TEMPORAL_RANGE filter + if query_object.filter: + for flt in query_object.filter: + if flt.get("op") == FilterOperator.TEMPORAL_RANGE: + col = flt.get("col") + if isinstance(col, str): + return col + elif isinstance(col, dict) and col.get("sqlExpression"): + return str(col.get("label") or col.get("sqlExpression")) + + # Strategy 2: Use explicitly set granularity if query_object.granularity: return query_object.granularity - # Strategy 2: Use x_axis_label if it exists + # Strategy 3: Use x_axis_label if it exists if x_axis_label: return x_axis_label - # Strategy 3: Find any datetime column in the datasource - if hasattr(self, "columns"): - for col in self.columns: - if hasattr(col, "is_dttm") and col.is_dttm: - if hasattr(col, "column_name"): - return col.column_name - elif hasattr(col, "name"): - return col.name - return None def _process_date_range_offset( diff --git a/tests/unit_tests/common/test_query_context_processor.py b/tests/unit_tests/common/test_query_context_processor.py index 0dfc47c4614..4c6dc386af4 100644 --- a/tests/unit_tests/common/test_query_context_processor.py +++ b/tests/unit_tests/common/test_query_context_processor.py @@ -440,11 +440,12 @@ def test_get_temporal_column_for_filter_with_x_axis_fallback(processor): def test_get_temporal_column_for_filter_with_datasource_columns(processor): - """Test _get_temporal_column_for_filter finds datetime column from datasource.""" + """Test _get_temporal_column_for_filter + returns None when no clear temporal column.""" query_object = MagicMock() query_object.granularity = None + query_object.filter = [] - # Mock datasource with datetime columns mock_datetime_col = MagicMock() mock_datetime_col.is_dttm = True mock_datetime_col.column_name = "created_at" @@ -459,21 +460,18 @@ def test_get_temporal_column_for_filter_with_datasource_columns(processor): query_object, None ) - assert result == "created_at" + assert result is None -def test_get_temporal_column_for_filter_with_datasource_name_attr(processor): - """Test _get_temporal_column_for_filter with columns using name attribute.""" +def test_get_temporal_column_for_filter_prefers_granularity(processor): + """Test _get_temporal_column_for_filter uses granularity when available.""" query_object = MagicMock() - query_object.granularity = None + query_object.granularity = "timestamp_col" + query_object.filter = [] - # Mock datasource with datetime column using 'name' attribute - # instead of 'column_name' mock_datetime_col = MagicMock() mock_datetime_col.is_dttm = True - mock_datetime_col.name = "timestamp_col" - # Remove column_name attribute to test name fallback - del mock_datetime_col.column_name + mock_datetime_col.name = "other_col" processor._qc_datasource.columns = [mock_datetime_col] diff --git a/tests/unit_tests/models/helpers_test.py b/tests/unit_tests/models/helpers_test.py index d1b5721822b..e913526975f 100644 --- a/tests/unit_tests/models/helpers_test.py +++ b/tests/unit_tests/models/helpers_test.py @@ -1522,6 +1522,68 @@ def test_adhoc_column_to_sqla_with_temporal_column_types(database: Database) -> assert "time_col" in result_str +def test_get_temporal_column_for_filter() -> None: + """Test _get_temporal_column_for_filter method with multiple strategies.""" + from superset.common.query_object import QueryObject + from superset.connectors.sqla.models import SqlaTable + from superset.utils.core import FilterOperator + + # Create a mock SqlaTable with columns + table = SqlaTable() + # Test Strategy 1: Use column from existing TEMPORAL_RANGE filter + query_object = QueryObject( + datasource=table, + filters=[ + { + "col": "date_column", + "op": FilterOperator.TEMPORAL_RANGE, + "val": "2024-01-01 : 2024-12-31", + } + ], + ) + result = table._get_temporal_column_for_filter(query_object, None) + assert result == "date_column" + + # Test Strategy 1 with dict column (sqlExpression) + query_object = QueryObject( + datasource=table, + filters=[ + { + "col": {"label": "custom_date", "sqlExpression": "DATE(created_at)"}, + "op": FilterOperator.TEMPORAL_RANGE, + "val": "2024-01-01 : 2024-12-31", + } + ], + ) + result = table._get_temporal_column_for_filter(query_object, None) + assert result == "custom_date" + + # Test Strategy 2: Use explicitly set granularity + query_object = QueryObject( + datasource=table, + granularity="created_at", + filters=[], + ) + result = table._get_temporal_column_for_filter(query_object, None) + assert result == "created_at" + + # Test Strategy 3: Use x_axis_label if it exists + query_object = QueryObject( + datasource=table, + filters=[], + ) + result = table._get_temporal_column_for_filter(query_object, "timestamp_col") + assert result == "timestamp_col" + + # Test no temporal column found + query_object = QueryObject( + datasource=table, + filters=[], + ) + result = table._get_temporal_column_for_filter(query_object, None) + assert result is None + + def test_adhoc_column_with_spaces_generates_quoted_sql(database: Database) -> None: """ Test that column names with spaces are properly quoted in the generated SQL.