Skip to content

Commit 8e69cda

Browse files
tlentotlento
authored
Merge pull request #871 from dbt-labs/fix-incorrect-cumulative-metric-output
2 parents 0d24aee + e628d6e commit 8e69cda

File tree

46 files changed

+1201
-39
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+1201
-39
lines changed

‎.changes/unreleased/Fixes-20231114-171137.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
kind: Fixes
2+
body: Fix error in cumulative metric output when start-time and end-time are specified
3+
time: 2023-11-14T17:11:37.462241-08:00
4+
custom:
5+
Author: tlento
6+
Issue: "869"

‎metricflow/filters/time_constraint.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ def _adjust_time_constraint_start_by_window(
6363
time_granularity: TimeGranularity,
6464
time_unit_count: int,
6565
) -> TimeRangeConstraint:
66-
"""Moves the start of the time constraint back by 1 window.
66+
"""Moves the start of the time constraint back by <time_unit_count> windows.
6767
6868
if the metric is weekly-active-users (ie window = 1 week) it moves time_constraint.start one week earlier
6969
"""
@@ -80,7 +80,7 @@ def adjust_time_constraint_for_cumulative_metric(
8080
) -> TimeRangeConstraint:
8181
"""Given a time constraint for the overall query, adjust it to cover the time range for this metric."""
8282
if granularity is not None:
83-
return self._adjust_time_constraint_start_by_window(granularity, count - 1)
83+
return self._adjust_time_constraint_start_by_window(granularity, count)
8484

8585
# if no window is specified we want to accumulate from the beginning of time
8686
return TimeRangeConstraint(

‎metricflow/test/integration/query_output/test_cumulative_metrics.py‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
from __future__ import annotations
2+
3+
import datetime
4+
5+
import pytest
6+
from _pytest.fixtures import FixtureRequest
7+
8+
from metricflow.engine.metricflow_engine import MetricFlowQueryRequest
9+
from metricflow.protocols.sql_client import SqlClient
10+
from metricflow.test.fixtures.setup_fixtures import MetricFlowTestSessionState
11+
from metricflow.test.integration.conftest import IntegrationTestHelpers
12+
from metricflow.test.snapshot_utils import assert_object_snapshot_equal
13+
14+
15+
@pytest.mark.sql_engine_snapshot
16+
def test_simple_cumulative_metric(
17+
request: FixtureRequest,
18+
mf_test_session_state: MetricFlowTestSessionState,
19+
sql_client: SqlClient,
20+
it_helpers: IntegrationTestHelpers,
21+
) -> None:
22+
"""Tests a query of a cumulative metric with a monthly window and a time constraint adjustment."""
23+
query_result = it_helpers.mf_engine.query(
24+
MetricFlowQueryRequest.create_with_random_request_id(
25+
metric_names=["trailing_2_months_revenue"],
26+
group_by_names=["metric_time"],
27+
order_by_names=["metric_time"],
28+
time_constraint_start=datetime.datetime(2020, 2, 1),
29+
time_constraint_end=datetime.datetime(2020, 4, 30),
30+
)
31+
)
32+
assert query_result.result_df is not None, "Unexpected empty result."
33+
34+
assert_object_snapshot_equal(
35+
request=request,
36+
mf_test_session_state=mf_test_session_state,
37+
obj_id="query_output",
38+
obj=query_result.result_df.to_string(),
39+
sql_client=sql_client,
40+
)
41+
42+
43+
@pytest.mark.sql_engine_snapshot
44+
def test_multiple_cumulative_metrics(
45+
request: FixtureRequest,
46+
mf_test_session_state: MetricFlowTestSessionState,
47+
sql_client: SqlClient,
48+
it_helpers: IntegrationTestHelpers,
49+
) -> None:
50+
"""Tests a query with multiple cumulative metrics to ensure date selections align."""
51+
query_result = it_helpers.mf_engine.query(
52+
MetricFlowQueryRequest.create_with_random_request_id(
53+
metric_names=["revenue_all_time", "trailing_2_months_revenue"],
54+
group_by_names=["metric_time"],
55+
order_by_names=["metric_time"],
56+
time_constraint_start=datetime.datetime(2020, 3, 31),
57+
time_constraint_end=datetime.datetime(2020, 5, 31),
58+
)
59+
)
60+
assert query_result.result_df is not None, "Unexpected empty result."
61+
62+
assert_object_snapshot_equal(
63+
request=request,
64+
mf_test_session_state=mf_test_session_state,
65+
obj_id="query_output",
66+
obj=query_result.result_df.to_string(),
67+
sql_client=sql_client,
68+
)
69+
70+
71+
@pytest.mark.sql_engine_snapshot
72+
def test_non_additive_cumulative_metric(
73+
request: FixtureRequest,
74+
mf_test_session_state: MetricFlowTestSessionState,
75+
sql_client: SqlClient,
76+
it_helpers: IntegrationTestHelpers,
77+
) -> None:
78+
"""Tests a query with a non-additive cumulative metric to ensure the non-additive constraint is applied."""
79+
query_result = it_helpers.mf_engine.query(
80+
MetricFlowQueryRequest.create_with_random_request_id(
81+
metric_names=["every_two_days_bookers"],
82+
group_by_names=["metric_time"],
83+
order_by_names=["metric_time"],
84+
time_constraint_start=datetime.datetime(2019, 12, 31),
85+
time_constraint_end=datetime.datetime(2020, 1, 3),
86+
)
87+
)
88+
assert query_result.result_df is not None, "Unexpected empty result."
89+
90+
assert_object_snapshot_equal(
91+
request=request,
92+
mf_test_session_state=mf_test_session_state,
93+
obj_id="query_output",
94+
obj=query_result.result_df.to_string(),
95+
sql_client=sql_client,
96+
)
97+
98+
99+
@pytest.mark.sql_engine_snapshot
100+
def test_grain_to_date_cumulative_metric(
101+
request: FixtureRequest,
102+
mf_test_session_state: MetricFlowTestSessionState,
103+
sql_client: SqlClient,
104+
it_helpers: IntegrationTestHelpers,
105+
) -> None:
106+
"""Tests a month to date cumulative metric with a constraint to ensure all necessary input data is included."""
107+
query_result = it_helpers.mf_engine.query(
108+
MetricFlowQueryRequest.create_with_random_request_id(
109+
metric_names=["revenue_mtd"],
110+
group_by_names=["metric_time"],
111+
order_by_names=["metric_time"],
112+
time_constraint_start=datetime.datetime(2021, 1, 3),
113+
time_constraint_end=datetime.datetime(2021, 1, 6),
114+
)
115+
)
116+
assert query_result.result_df is not None, "Unexpected empty result."
117+
118+
assert_object_snapshot_equal(
119+
request=request,
120+
mf_test_session_state=mf_test_session_state,
121+
obj_id="query_output",
122+
obj=query_result.result_df.to_string(),
123+
sql_client=sql_client,
124+
)
125+
126+
127+
@pytest.mark.sql_engine_snapshot
128+
def test_cumulative_metric_with_non_adjustable_filter(
129+
request: FixtureRequest,
130+
mf_test_session_state: MetricFlowTestSessionState,
131+
sql_client: SqlClient,
132+
it_helpers: IntegrationTestHelpers,
133+
) -> None:
134+
"""Tests a cumulative metric with a filter that cannot be adjusted to ensure all data is included."""
135+
query_result = it_helpers.mf_engine.query(
136+
MetricFlowQueryRequest.create_with_random_request_id(
137+
metric_names=["trailing_2_months_revenue"],
138+
group_by_names=["metric_time"],
139+
order_by_names=["metric_time"],
140+
where_constraint=(
141+
"{{ TimeDimension('metric_time', 'day') }} = '2020-03-15' or "
142+
"{{ TimeDimension('metric_time', 'day') }} = '2020-04-30'"
143+
),
144+
)
145+
)
146+
assert query_result.result_df is not None, "Unexpected empty result."
147+
148+
assert_object_snapshot_equal(
149+
request=request,
150+
mf_test_session_state=mf_test_session_state,
151+
obj_id="query_output",
152+
obj=query_result.result_df.to_string(),
153+
sql_client=sql_client,
154+
)

‎metricflow/test/integration/test_cases/itest_cumulative_metric.yaml‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ integration_test:
66
metrics: ["trailing_2_months_revenue"]
77
group_bys: ["metric_time"]
88
order_bys: ["metric_time"]
9-
time_constraint: ["2020-01-05", "2021-01-04"]
9+
time_constraint: ["2020-03-05", "2021-01-04"]
1010
check_query: |
1111
SELECT
1212
SUM(b.txn_revenue) as trailing_2_months_revenue
@@ -23,6 +23,7 @@ integration_test:
2323
FROM {{ source_schema }}.fct_revenue
2424
) b
2525
ON b.ds <= a.ds AND b.ds > {{ render_date_sub("a", "ds", 2, TimeGranularity.MONTH) }}
26+
WHERE {{ render_time_constraint("a.ds", "2020-03-05", "2021-01-04") }}
2627
GROUP BY a.ds
2728
ORDER BY a.ds
2829
---
@@ -33,7 +34,7 @@ integration_test:
3334
metrics: ["trailing_2_months_revenue"]
3435
group_bys: ["metric_time", "user__home_state_latest"]
3536
order_bys: ["metric_time", "user__home_state_latest"]
36-
time_constraint: ["2020-01-05", "2021-01-04"]
37+
time_constraint: ["2020-03-05", "2021-01-04"]
3738
check_query: |
3839
SELECT
3940
SUM(revenue) as trailing_2_months_revenue
@@ -55,6 +56,7 @@ integration_test:
5556
GROUP BY m.created_at, m.revenue, u.home_state_latest
5657
) b
5758
ON b.ds <= a.ds AND b.ds > {{ render_date_sub("a", "ds", 2, TimeGranularity.MONTH) }}
59+
WHERE {{ render_time_constraint("a.ds", "2020-03-05", "2021-01-04") }}
5860
GROUP BY a.ds, user__home_state_latest
5961
ORDER by a.ds, user__home_state_latest
6062
---
@@ -65,15 +67,15 @@ integration_test:
6567
metrics: ["revenue_all_time"]
6668
group_bys: ["metric_time"]
6769
order_bys: ["metric_time"]
68-
time_constraint: ["2020-01-01", "2021-01-05"]
70+
time_constraint: ["2020-03-01", "2021-01-05"]
6971
check_query: |
7072
SELECT
7173
SUM(revenue) AS revenue_all_time
7274
, a.ds AS metric_time__day
7375
FROM (
7476
SELECT ds
7577
FROM {{ mf_time_spine_source }}
76-
WHERE {{ render_time_constraint("ds", "2020-01-01", "2021-01-05") }}
78+
WHERE {{ render_time_constraint("ds", "2020-03-01", "2021-01-05") }}
7779
) a
7880
INNER JOIN (
7981
SELECT
@@ -102,7 +104,7 @@ integration_test:
102104
metrics: ["revenue_all_time", "trailing_2_months_revenue"]
103105
group_bys: ["metric_time"]
104106
order_bys: ["metric_time"]
105-
time_constraint: ["2019-12-31", "2021-01-05"]
107+
time_constraint: ["2020-03-31", "2021-01-05"]
106108
check_query: |
107109
SELECT revenue_all_time, trailing_2_months_revenue, a.ds AS metric_time__day
108110
FROM (
@@ -116,7 +118,7 @@ integration_test:
116118
FROM (
117119
SELECT ds
118120
FROM {{ mf_time_spine_source }}
119-
WHERE {{ render_time_constraint("ds", "2019-12-31", "2021-01-05") }}
121+
WHERE {{ render_time_constraint("ds", "2020-03-31", "2021-01-05") }}
120122
) a
121123
INNER JOIN (
122124
SELECT
@@ -135,7 +137,7 @@ integration_test:
135137
FROM (
136138
SELECT ds
137139
FROM {{ mf_time_spine_source }}
138-
WHERE {{ render_time_constraint("ds", "2019-12-31", "2021-01-05") }}
140+
WHERE {{ render_time_constraint("ds", "2020-01-31", "2021-01-05") }}
139141
) a
140142
INNER JOIN (
141143
SELECT
@@ -147,6 +149,7 @@ integration_test:
147149
GROUP BY a.ds
148150
) b
149151
ON a.ds = b.ds
152+
WHERE {{ render_time_constraint("a.ds", "2020-03-31", "2021-01-05") }}
150153
ORDER BY a.ds
151154
---
152155
integration_test:
@@ -156,7 +159,7 @@ integration_test:
156159
metrics: ["trailing_2_months_revenue"]
157160
group_bys: ["metric_time__day"]
158161
order_bys: ["metric_time__day"]
159-
time_constraint: ["2020-01-05", "2021-01-04"]
162+
time_constraint: ["2020-03-05", "2021-01-04"]
160163
check_query: |
161164
SELECT
162165
SUM(b.txn_revenue) as trailing_2_months_revenue
@@ -173,6 +176,7 @@ integration_test:
173176
FROM {{ source_schema }}.fct_revenue
174177
) b
175178
ON b.ds <= a.ds AND b.ds > {{ render_date_sub("a", "ds", 2, TimeGranularity.MONTH) }}
179+
WHERE {{ render_time_constraint("a.ds", "2020-03-05", "2021-01-04") }}
176180
GROUP BY a.ds
177181
ORDER BY a.ds
178182
---
@@ -218,7 +222,7 @@ integration_test:
218222
FROM (
219223
SELECT ds
220224
FROM {{ mf_time_spine_source }}
221-
WHERE {{ render_time_constraint("ds", "2019-12-31", "2020-01-04") }}
225+
WHERE {{ render_time_constraint("ds", "2019-12-30", "2020-01-04") }}
222226
) a
223227
INNER JOIN (
224228
SELECT
@@ -237,15 +241,15 @@ integration_test:
237241
metrics: ["every_two_days_bookers"]
238242
group_bys: ["metric_time"]
239243
order_bys: ["metric_time"]
240-
time_constraint: ["2020-01-04", "2020-01-04"]
244+
time_constraint: ["2020-01-03", "2020-01-03"]
241245
check_query: |
242246
SELECT
243247
COUNT (DISTINCT(b.guest_id)) as every_two_days_bookers
244248
, a.ds AS metric_time__day
245249
FROM (
246250
SELECT ds
247251
FROM {{ mf_time_spine_source }}
248-
WHERE {{ render_time_constraint("ds", "2020-01-04", "2020-01-04") }}
252+
WHERE {{ render_time_constraint("ds", "2020-01-02", "2020-01-03") }}
249253
) a
250254
INNER JOIN (
251255
SELECT
@@ -254,6 +258,7 @@ integration_test:
254258
FROM {{ source_schema }}.fct_bookings
255259
) b
256260
ON b.ds <= a.ds AND b.ds > {{ render_date_sub("a", "ds", 2, TimeGranularity.DAY) }}
261+
WHERE {{ render_time_constraint("a.ds", "2020-01-03", "2020-01-03") }}
257262
GROUP BY a.ds
258263
ORDER BY a.ds
259264
---
@@ -264,15 +269,15 @@ integration_test:
264269
metrics: ["revenue_mtd"]
265270
group_bys: ["metric_time"]
266271
order_bys: ["metric_time"]
267-
time_constraint: ["2020-01-01", "2021-01-05"]
272+
time_constraint: ["2021-01-04", "2021-01-05"]
268273
check_query: |
269274
SELECT
270275
SUM(b.txn_revenue) as revenue_mtd
271276
, a.ds AS metric_time__day
272277
FROM (
273278
SELECT ds
274279
FROM {{ mf_time_spine_source }}
275-
WHERE {{ render_time_constraint("ds", "2020-01-01", "2021-01-05") }}
280+
WHERE {{ render_time_constraint("ds", "2021-01-01", "2021-01-05") }}
276281
) a
277282
INNER JOIN (
278283
SELECT
@@ -281,6 +286,7 @@ integration_test:
281286
FROM {{ source_schema }}.fct_revenue
282287
) b
283288
ON b.ds <= a.ds AND b.ds >= {{ render_date_trunc("a.ds", TimeGranularity.MONTH) }}
289+
WHERE {{ render_time_constraint("a.ds", "2021-01-04", "2021-01-05") }}
284290
GROUP BY a.ds
285291
ORDER BY a.ds
286292
---

‎metricflow/test/snapshots/test_cumulative_metric_rendering.py/SqlQueryPlan/BigQuery/test_cumulative_metric_with_time_constraint__plan0.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ FROM (
6565
WHERE subq_4.ds BETWEEN '2020-01-01' AND '2020-01-01'
6666
) subq_3
6767
INNER JOIN (
68-
-- Constrain Time Range to [2019-12-01T00:00:00, 2020-01-01T00:00:00]
68+
-- Constrain Time Range to [2019-11-01T00:00:00, 2020-01-01T00:00:00]
6969
SELECT
7070
subq_1.ds__day
7171
, subq_1.ds__week
@@ -173,7 +173,7 @@ FROM (
173173
FROM ***************************.fct_revenue revenue_src_10006
174174
) subq_0
175175
) subq_1
176-
WHERE subq_1.metric_time__day BETWEEN '2019-12-01' AND '2020-01-01'
176+
WHERE subq_1.metric_time__day BETWEEN '2019-11-01' AND '2020-01-01'
177177
) subq_2
178178
ON
179179
(

‎metricflow/test/snapshots/test_cumulative_metric_rendering.py/SqlQueryPlan/BigQuery/test_cumulative_metric_with_time_constraint__plan0_optimized.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ FROM (
1717
INNER JOIN (
1818
-- Read Elements From Semantic Model 'revenue'
1919
-- Metric Time Dimension 'ds'
20-
-- Constrain Time Range to [2019-12-01T00:00:00, 2020-01-01T00:00:00]
20+
-- Constrain Time Range to [2019-11-01T00:00:00, 2020-01-01T00:00:00]
2121
SELECT
2222
DATE_TRUNC(created_at, day) AS metric_time__day
2323
, DATE_TRUNC(created_at, month) AS metric_time__month
2424
, revenue AS txn_revenue
2525
FROM ***************************.fct_revenue revenue_src_10006
26-
WHERE DATE_TRUNC(created_at, day) BETWEEN '2019-12-01' AND '2020-01-01'
26+
WHERE DATE_TRUNC(created_at, day) BETWEEN '2019-11-01' AND '2020-01-01'
2727
) subq_11
2828
ON
2929
(

‎metricflow/test/snapshots/test_cumulative_metric_rendering.py/SqlQueryPlan/Databricks/test_cumulative_metric_with_time_constraint__plan0.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ FROM (
6565
WHERE subq_4.ds BETWEEN '2020-01-01' AND '2020-01-01'
6666
) subq_3
6767
INNER JOIN (
68-
-- Constrain Time Range to [2019-12-01T00:00:00, 2020-01-01T00:00:00]
68+
-- Constrain Time Range to [2019-11-01T00:00:00, 2020-01-01T00:00:00]
6969
SELECT
7070
subq_1.ds__day
7171
, subq_1.ds__week
@@ -173,7 +173,7 @@ FROM (
173173
FROM ***************************.fct_revenue revenue_src_10006
174174
) subq_0
175175
) subq_1
176-
WHERE subq_1.metric_time__day BETWEEN '2019-12-01' AND '2020-01-01'
176+
WHERE subq_1.metric_time__day BETWEEN '2019-11-01' AND '2020-01-01'
177177
) subq_2
178178
ON
179179
(

0 commit comments

Comments
 (0)