mikebridge commented on code in PR #44997:
URL: https://github.com/apache/superset/pull/44997#discussion_r4209118932
##########
superset/semantic_layers/models.py:
##########
@@ -595,6 +595,18 @@ def column_names(self) -> list[str]:
def data(self) -> ExplorableData:
dimensions = self._unique_dimensions
metrics = list(self.implementation.get_metrics())
+ preferred_time_dimension: str | None = (
+ self.implementation.preferred_time_dimension
+ )
+ main_dttm_col: str | None = next(
Review Comment:
Good catch, the payload-only value left backend consumers without the
preference. `2260340fa3` adds a validated `main_dttm_col` property on the model
that both backend lookup and the Explore payload now use. The preference is
accepted only when it names an exposed temporal dimension. The focused tests
cover a preferred temporal dimension and invalid or absent preferences.
##########
superset/semantic_layers/models.py:
##########
@@ -595,6 +595,18 @@ def column_names(self) -> list[str]:
def data(self) -> ExplorableData:
dimensions = self._unique_dimensions
metrics = list(self.implementation.get_metrics())
+ preferred_time_dimension: str | None = (
+ self.implementation.preferred_time_dimension
+ )
+ main_dttm_col: str | None = next(
+ (
+ dimension.name
+ for dimension in dimensions
+ if dimension.name == preferred_time_dimension
+ and get_column_type(dimension.type) == GenericDataType.TEMPORAL
+ ),
+ None,
Review Comment:
Good catch, `2260340fa3` logs a warning when a configured preference does
not match an exposed temporal dimension and falls back to no default, while
leaving an unset preference quiet.
##########
superset/semantic_layers/models.py:
##########
@@ -679,7 +691,7 @@ def data(self) -> ExplorableData:
# sql-specific
"schema": None,
"catalog": None,
- "main_dttm_col": None,
+ "main_dttm_col": main_dttm_col,
Review Comment:
Thanks for flagging the mismatch. The compatibility helper appears able to
preserve a valid column despite empty choices, so clearing is not inevitable,
but the time selector did lack its semantic-view options. `2260340fa3` now
serializes the exposed temporal dimensions as `granularity_sqla` choices, and
the model test checks that the preferred value and the choices agree.
--
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]
