Support conversion metrics with custom grain #1475

courtneyholcomb · 2024-10-23T22:29:39Z

Support for conversion metrics used with custom granularities in the group by or the where filter.

courtneyholcomb · 2024-10-29T01:27:52Z

metricflow-semantics/metricflow_semantics/specs/spec_set.py

-from metricflow_semantics.specs.metric_spec import MetricSpec
-from metricflow_semantics.specs.time_dimension_spec import TimeDimensionSpec
+
+if TYPE_CHECKING:


Needed to resolve circular imports.

Adds all_linkable_specs_required_for_source_nodes property. Storing this on SourceNodeRecipe allows us to keep logic for converting to base grain in one place instead of needing to propogate that in several places and add a comment explaining each one. This commit also adds a little bit of type cleanup to the same class.

courtneyholcomb · 2024-10-29T01:50:10Z

...y.py/SqlQueryPlan/DuckDB/test_conversion_metric_with_custom_granularity__plan0_optimized.sql

+          ORDER BY subq_28.ds__day DESC
+          ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
+        ) AS ds__day
+        , FIRST_VALUE(subq_28.metric_time__day) OVER (


Is it expected that we still use metric_time__day in the interim calculations here? Looks like we use both metric_time__day and metric_time__martian_day in the window functions, even though only metric_time__martian_day was in the group by.

how does cumulative windows work for custom grains? We have metric_time__day here because that's what's used for the conversion window calculation (ie., look at line 126 below for the join condition). So metric_time__day is here because it's used for the conversion window and metric_time__martian_day is there because we will need it when grouping by it.

actually, looking at this, we use ds__day to do the calculation. There's probably some wonky thing happening with the column's I passed into the conversion node, not blocking, but I'll take a look.

Cumulative windows can't use custom grains yet - that's part of the implementation we were discussing at MF team meeting today!
I think non-default grains are relevant, though. If you look at this SQL snapshot you can see an example of the window function we implement for that.

But it sounds like the PR you put up today was a fix for what you mentioned here!

WilliamDee · 2024-10-29T20:11:20Z

...y.py/SqlQueryPlan/DuckDB/test_conversion_metric_with_custom_granularity__plan0_optimized.sql

+          ORDER BY subq_28.ds__day DESC
+          ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING
+        ) AS ds__day
+        , FIRST_VALUE(subq_28.metric_time__day) OVER (


how does cumulative windows work for custom grains? We have metric_time__day here because that's what's used for the conversion window calculation (ie., look at line 126 below for the join condition). So metric_time__day is here because it's used for the conversion window and metric_time__martian_day is there because we will need it when grouping by it.

WilliamDee · 2024-10-29T20:13:15Z

.changes/unreleased/Features-20241023-161720.yaml

@@ -0,0 +1,6 @@
+kind: Features
+body: Support cumulative metrics queried with custom granularities.


Support conversion** metrics

WilliamDee · 2024-10-29T20:14:25Z

tests_metricflow/integration/test_cases/itest_granularity.yaml

+              {{ render_date_trunc("ds", TimeGranularity.DAY) }} AS ds__day
+              , user_id AS user
+              , 1 AS buys
+              , GEN_RANDOM_UUID() AS mf_internal_uuid


Need to swap GEN_RANDOM_UUID() with {{ generate_random_uuid() }}

cla-bot bot added the cla:yes label Oct 23, 2024

courtneyholcomb force-pushed the court/custom-conversion branch from ec3faac to c93914d Compare October 23, 2024 23:15

courtneyholcomb changed the base branch from main to will/fix-extra-linkable-specs October 23, 2024 23:16

dbt-labs deleted a comment from github-actions bot Oct 23, 2024

courtneyholcomb force-pushed the court/custom-conversion branch from 2d1d31a to 57228ab Compare October 24, 2024 17:30

WilliamDee force-pushed the will/fix-extra-linkable-specs branch from 98d16f5 to 7aa19d7 Compare October 24, 2024 19:17

Base automatically changed from will/fix-extra-linkable-specs to main October 25, 2024 02:40

courtneyholcomb force-pushed the court/custom-conversion branch from 57228ab to ba8bdb2 Compare October 25, 2024 20:22

Add helper

38df97a

courtneyholcomb force-pushed the court/custom-conversion branch 2 times, most recently from a470bdb to fc171b4 Compare October 29, 2024 01:23

courtneyholcomb commented Oct 29, 2024

View reviewed changes

Cleanup

fb562aa

courtneyholcomb force-pushed the court/custom-conversion branch 2 times, most recently from ab0e7fa to 9a41e79 Compare October 29, 2024 01:34

courtneyholcomb force-pushed the court/custom-conversion branch 2 times, most recently from 7576c0e to b636dd6 Compare October 29, 2024 01:40

courtneyholcomb added 2 commits October 28, 2024 18:42

Add custom grain logic for conversion metrics

1a6d623

Add query rendering tests for conversion metrics with custom grain

876daf2

courtneyholcomb force-pushed the court/custom-conversion branch from b636dd6 to 1cd7601 Compare October 29, 2024 01:43

courtneyholcomb commented Oct 29, 2024

View reviewed changes

Add SQL engine snapshots

55dd1ba

courtneyholcomb force-pushed the court/custom-conversion branch from 1cd7601 to 22253ce Compare October 29, 2024 02:01

courtneyholcomb marked this pull request as ready for review October 29, 2024 02:01

courtneyholcomb requested a review from WilliamDee October 29, 2024 02:01

courtneyholcomb added the Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment label Oct 29, 2024

courtneyholcomb had a problem deploying to DW_INTEGRATION_TESTS October 29, 2024 02:01 — with GitHub Actions Failure

courtneyholcomb added 2 commits October 28, 2024 19:08

Check query tests for conversion metrics with custom grain

5c55bbc

Changelog

05aed81

courtneyholcomb force-pushed the court/custom-conversion branch from 22253ce to 05aed81 Compare October 29, 2024 02:08

courtneyholcomb added Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment and removed Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment labels Oct 29, 2024

courtneyholcomb had a problem deploying to DW_INTEGRATION_TESTS October 29, 2024 02:09 — with GitHub Actions Failure

github-actions bot removed the Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment label Oct 29, 2024

WilliamDee approved these changes Oct 29, 2024

View reviewed changes

WilliamDee reviewed Oct 29, 2024

View reviewed changes

PR feedback - cleanup

64f6623

courtneyholcomb added the Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment label Oct 30, 2024

courtneyholcomb temporarily deployed to DW_INTEGRATION_TESTS October 30, 2024 01:06 — with GitHub Actions Inactive

courtneyholcomb temporarily deployed to DW_INTEGRATION_TESTS October 30, 2024 01:07 — with GitHub Actions Inactive

github-actions bot removed the Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment label Oct 30, 2024

courtneyholcomb merged commit c611b44 into main Oct 30, 2024
29 checks passed

courtneyholcomb deleted the court/custom-conversion branch October 30, 2024 01:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support conversion metrics with custom grain #1475

Support conversion metrics with custom grain #1475

courtneyholcomb commented Oct 23, 2024 •

edited

Loading

courtneyholcomb Oct 29, 2024

courtneyholcomb Oct 29, 2024

WilliamDee Oct 29, 2024

WilliamDee Oct 29, 2024

courtneyholcomb Oct 30, 2024

courtneyholcomb Oct 30, 2024

WilliamDee Oct 29, 2024

WilliamDee Oct 29, 2024

WilliamDee Oct 29, 2024

		@@ -0,0 +1,6 @@
		kind: Features
		body: Support cumulative metrics queried with custom granularities.

Support conversion metrics with custom grain #1475

Support conversion metrics with custom grain #1475

Conversation

courtneyholcomb commented Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

courtneyholcomb commented Oct 23, 2024 •

edited

Loading