dbt-labs / metricflow

MetricFlow allows you to define, build, and maintain metrics in code.
https://docs.getdbt.com/docs/build/about-metricflow
Other
1.12k stars 92 forks source link

Use `MetricTimeDefaultGranularityPattern` to resolve `metric_time` granularity #1332

Closed courtneyholcomb closed 1 month ago

courtneyholcomb commented 1 month ago

Resolve metric time using new spec pattern in group by and filter nodes. This is a change from defaulting to the minimum time granularity for metric_time. Now we will use the max default granularity for the requested metrics.

Note the snapshot updates. These reflect another behavior change in which we don't error if metric_time is queried for metrics with two different default granularities. Instead, we choose the larger of the two, which is guaranteed to work for both metrics.

courtneyholcomb commented 1 month ago

Changes look good, but I am wondering about the behavior change for how the default now allows for what was previously an error condition. From what I remember, we wanted to error out to make it easier to see from the configs what the granularity for a time dimension was. Has that thought changed from the product side?

@plypaul this is my understanding of the behavior change (mentioned in the PR description!). This scenario occurs if you query multiple metrics or a derived metric with different agg_time_dimensions. That means there are multiple default granularities for those metrics/input metrics.