dbt-labs / dbt-labs/metricflow
TimeSpineRule warns on any time dimension granularity, even ones never used as agg_time_dimension
- Dominant language
- Python
- Stars
- 1.8k
- Forks
- 202
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 14
Description
### Describe the bug
`TimeSpineRule.validate_manifest` (`metricflow_semantic_interfaces/validations/time_spines.py`) compares the smallest granularity across **every** time-typed dimension in the manifest against the smallest configured time spine, and emits a `ValidationWarning` if any dimension is finer-grained than the finest time spine:
```python
dimension_granularities = {
dimension.type_params.time_granularity
for semantic_model in semantic_manifest.semantic_models
for dimension in semantic_model.dimensions
if dimension.type_params
}
...
smallest_dim_granularity = min(dimension_granularities)
smallest_time_spine_granularity = min(time_spines_by_granularity.keys())
if smallest_dim_granularity < smallest_time_spine_granularity:
issues.append(ValidationWarning(...))
```
This check doesn't distinguish between:
- a dimension that is (or could be) used as an `agg_time_dimension` and therefore can actually be joined to a time spine via `JoinToTimeSpineNode` (cumulative metrics, `fill_nulls_with`, offset windows), vs.
- a plain time-typed dimension that is never an `agg_time_dimension` and is only ever used for filtering (e.g. an exact `last_modified_at` timestamp with `time_granularity: second` used for freshness checks).
In the second case, the warning fires even though a finer-grained time spine will never actually be needed, since spine joins are only built off a metric's `agg_time_dimension`.
### Expected behavior
The check should only consider dimensions that are actually referenced as an `agg_time_dimension` (via `semantic_model.defaults.agg_time_dimension` and/or `checked_agg_time_dimension_for_measure`), since those are the only dimensions whose granularity can trigger a real time-spine join gap.
### Impact / workaround
There's currently no way to suppress this per-dimension or per-model — `warn_error_options.silence` only matches on event class name (`SemanticValidationFailure`), so silencing it also suppresses unrelated, legitimate semantic-manifest warnings (duplicate time-spine granularities, "no time dimensions configured," etc.) project-wide.
### Repro sketch
- Semantic model with `standard_granularity_column` at `day` grain (project time spine: `day`).
- A time dimension in that model (or another model) with `type_params.time_granularity: second`, never set as `agg_time_dimension`, never used in a cumulative/`fill_nulls_with`/offset metric.
- `mf validate-configs` (or manifest parse) emits: `"To avoid unexpected query errors, configuring a time spine at or below the smallest time dimension granularity is recommended. Smallest time dimension granularity: SECOND; Smallest time spine granularity: DAY"` — even though no query path will ever need a second-grain spine for that dimension.
Contributor guide
Assessment
This issue has not been assessed yet.