feat: fixed_grain for level metrics - #2567
Merged
Merged
Conversation
betodealmeida
marked this pull request as ready for review
September 23, 2026 13:38
betodealmeida
force-pushed
the
semi-additive-metrics
branch
from
September 23, 2026 14:03
57ddf78 to
1f22459
Compare
betodealmeida
force-pushed
the
robind/fixed-grain
branch
2 times, most recently
from
September 23, 2026 16:29
c361187 to
cb505e0
Compare
An experimentation metric is a ratio of two unit-grain metrics: the activity each allocated unit contributed, over the allocated population. Writing that shape down here records where this branch stops. Two refusals land on the shape rather than on an edge case. A population written as COUNT(DISTINCT unit) at unit grain is refused, though at unit grain the count is one per unit and the merge is a plain sum. And the ratio always spans two facts, since the population comes from the allocation table, so the cross-fact refusal applies to every one of these metrics. The global grain the refusal suggests does build, and answers a different question: OVER () broadcasts the total across every group, so the result divides an all-time total by one month's population. The last test is xfail rather than an assertion on the generated SQL. A fixed-grain dimension outside the query grain never reaches the measures CTE, so the window partitions by a column that does not exist. Pinning that SQL would lock in something no engine accepts, so it asserts only the invariant: a column read from a CTE has to be projected by it.
betodealmeida
force-pushed
the
robind/fixed-grain
branch
from
September 23, 2026 21:27
cb505e0 to
a974caa
Compare
✅ Deploy Preview for thriving-cassata-78ae72 canceled.
|
shangyian
approved these changes
Sep 24, 2026
Collaborator
|
Exciting! Going to merge so we can release 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A rebase of @robinld's #2532 onto my PR #2502 after I addressed all the comments from @shangyian and @robinld.
Test Plan
make checkpassesmake testshows 100% unit test coverageDeployment Plan