Vitor-Avila commented on code in PR #36309:
URL: https://github.com/apache/superset/pull/36309#discussion_r2571968258
##########
superset/models/helpers.py:
##########
@@ -1698,31 +1698,32 @@ def _get_temporal_column_for_filter( # noqa: C901
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"))
Review Comment:
When would `col` be a `dict`? From my tests, if you use custom SQL to create
the original time range filter, the chart won't allow you to add a time
comparison config. You can use a calculated column, but looking at
`queries[0].filter` I still see this in the payload:
```
{col: "${COLUMN_NAME}", op: "TEMPORAL_RANGE", val: "2005 : 2006"}
```
Is this scenario a precautionary measure, or something you were able to
reproduce? I'm asking it because I noticed you're looking for `label` or
`sqlExpression`, and I was wondering if we should look for `name` too (which I
think it depends if this is for a particular calculated column flow I couldn't
reproduce, or specifically for custom SQL in the filter).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
