shoemoney commented on code in PR #44460:
URL: https://github.com/apache/superset/pull/44460#discussion_r4068051407


##########
superset/common/query_context_factory.py:
##########
@@ -275,6 +275,39 @@ def _apply_granularity(  # noqa: C901
             query_object.granularity = main_dttm_col
             return
 
+        if (
+            not x_axis
+            and query_object.granularity is None
+            and query_object.is_timeseries
+        ):
+            # A chart saved before the x-axis control existed keeps its time
+            # column in ``form_data`` under the legacy ``granularity_sqla`` key
+            # and never wrote it into the stored query object. The paths that
+            # rebuild a query from form data resolve that key
+            # (``extractExtras.ts`` for Explore, ``form_data_query_context`` 
for
+            # the Excel export and MCP tools), but anything that replays the
+            # stored ``query_context`` verbatim reaches the
+            # ``not granularity and is_timeseries`` guard in ``models/helpers``
+            # and fails with "Datetime column not provided as part table
+            # configuration". Resolving the legacy key here, and only then the
+            # dataset's main datetime column, gives both kinds of consumer the
+            # same time subject. Candidates are matched against the dataset's
+            # temporal columns so one that has since been dropped, or is no
+            # longer temporal, is ignored.
+            candidates = (
+                (form_data or {}).get("granularity_sqla"),
+                (form_data or {}).get("granularity"),
+                getattr(datasource, "main_dttm_col", None),
+            )
+            query_object.granularity = next(
+                (
+                    candidate
+                    for candidate in candidates
+                    if candidate in temporal_columns
+                ),
+                None,
+            )

Review Comment:
   The fallback still reached the later first-temporal-filter removal in 
cafc5bb. I reproduced it with a legacy `granularity_sqla=ds` chart and an 
independent `event_end` range: the filter was deleted.
   
   Fixed in d5d5068151 by returning after legacy granularity inference. The 
regression fails on cafc5bb and passes with the fix; all 338 common unit tests 
and the staged pre-commit hooks (including mypy, Ruff and Pylint) pass.



-- 
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]

Reply via email to