eschutho commented on code in PR #41456:
URL: https://github.com/apache/superset/pull/41456#discussion_r3547648537
##########
superset/semantic_layers/mapper.py:
##########
@@ -337,18 +337,43 @@ def map_query_object(query_object: ValidatedQueryObject)
-> list[SemanticQuery]:
metrics = [all_metrics[metric] for metric in (query_object.metrics or [])]
grain = _convert_time_grain(query_object.extras.get("time_grain_sqla"))
- dimensions = [
- dimension
- for dimension in semantic_view.dimensions
- if dimension.name in normalized_columns
- and (
- # if a grain is specified, only include the time dimension if its
grain
- # matches the requested grain
- grain is None
- or dimension.name != query_object.granularity
- or dimension.grain == grain
+ time_axis_column = _get_time_axis_column(query_object, all_dimensions)
+ # A semantic view can expose multiple Dimension variants per name (one per
+ # supported time grain). Pick exactly one variant per selected column:
+ # for the time-axis column we honor the user's grain selection, falling
+ # back to the raw / no-grain variant when no exact match exists and then
+ # to any available variant so the axis is never silently dropped; for
+ # every other selected column we prefer the raw variant and otherwise
+ # take any available variant.
+ dimensions: list[Dimension] = []
+ seen_non_axis: dict[str, Dimension] = {}
+ axis_variants: list[Dimension] = []
+ axis_match: Dimension | None = None
+ for dimension in semantic_view.dimensions:
+ if dimension.name not in normalized_columns:
+ continue
+ if dimension.name == time_axis_column:
+ axis_variants.append(dimension)
+ if axis_match is None and dimension.grain == grain:
+ axis_match = dimension
+ continue
+ existing = seen_non_axis.get(dimension.name)
+ if existing is None or (existing.grain is not None and dimension.grain
is None):
+ seen_non_axis[dimension.name] = dimension
+
+ if axis_match is not None:
+ dimensions.append(axis_match)
+ elif axis_variants:
+ # No variant matches the requested grain. Prefer the raw (grain=None)
+ # variant; otherwise pick a deterministic fallback so the axis stays
+ # on the query instead of being silently dropped.
+ raw_variant = next((v for v in axis_variants if v.grain is None), None)
+ dimensions.append(
+ raw_variant
+ if raw_variant is not None
+ else min(axis_variants, key=lambda v: v.grain.name if v.grain else
"")
Review Comment:
**superset/semantic_layers/mapper.py:366-374**
When no time grain is requested at all and the time-axis column exposes only
grained `Dimension` variants (no `grain=None` raw one), `axis_match` stays
`None` here and the code falls through to `min(axis_variants, key=lambda v:
v.grain.name ...)` — silently substituting an alphabetically-picked grain (e.g.
"Day") for what should be raw data. `_validate_granularity` doesn't catch this
since its grain-compatibility check only runs when a grain is actually
requested.
Is it possible for a semantic view's time dimension to expose only grained
variants with no raw one, or does the protocol guarantee a raw variant always
exists? If it's possible, would it be worth surfacing an error (or defaulting
more conservatively) instead of picking a grain silently here? WDYT?
_This comment was generated by Claude (AI) on behalf of @eschutho._
--
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]
