dbt-labs / dbt-labs/metricflow

TimeSpineRule warns on any time dimension granularity, even ones never used as agg_time_dimension

Open
#2,115 1 comment 0 reactions 0 assignees View on GitHub
bug Metricflow
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.