-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SQL rendering tests for metric queries with custom granularity #1410
Conversation
889ba84
to
728e2c8
Compare
ebb64c0
to
7ce16d1
Compare
728e2c8
to
3ed4cbc
Compare
7ce16d1
to
24d77fd
Compare
3ed4cbc
to
85f96c1
Compare
bd5d6b1
to
c6b4526
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! One note - we need to explicitly test all the time spine/fill nulls scenarios, and maybe some offset metrics, but I assume the former is not implemented yet and the latter might be more valuable as part of a test suite when we begin supporting custom granularity windows. The base case (like with cumulative) might be nice to have from the beginning, though.
Render SQL for custom granularities. The `JoinToCustomGranularityNode` will build a time spine dataset and `LEFT OUTER JOIN` it to the parent dataset, without a subquery. This is done by joining the time spine table directly, instead of creating a subquery that selects from the time spine table and then joins. That makes this the first node that does not create its own subquery. This join should be safe to do without a subquery since it is always a `LEFT OUTER JOIN`, ensuring the number of rows will not be changed by the join, making it relatively safe for optimizers to handle. This behavior was discussed with @tlento and agreed upon in order to ensure optimization and readability. You can see examples of what this SQL will look like in a [testing PR](#1410) higher up the stack.
3050414
to
704e79a
Compare
c6b4526
to
daf40f9
Compare
SQL rendering tests for querying metrics with custom granularities. Does not include tests with filters (coming later). If you have suggestions for additional metric scenarios that should be tested, please let me know!