Skip to content
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

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

courtneyholcomb
Copy link
Contributor

@courtneyholcomb courtneyholcomb commented Sep 18, 2024

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!

@cla-bot cla-bot bot added the cla:yes label Sep 18, 2024
@dbt-labs dbt-labs deleted a comment from github-actions bot Sep 18, 2024
@cla-bot cla-bot bot added the cla:yes label Sep 18, 2024
@courtneyholcomb courtneyholcomb force-pushed the court/custom-grain-dfp-7 branch 2 times, most recently from ebb64c0 to 7ce16d1 Compare September 18, 2024 22:20
@courtneyholcomb courtneyholcomb changed the title Write various tests for metric queries with custom granularity Write tests for metric queries with custom granularity Sep 18, 2024
@courtneyholcomb courtneyholcomb marked this pull request as ready for review September 18, 2024 22:21
@courtneyholcomb courtneyholcomb changed the title Write tests for metric queries with custom granularity Write SQL rendering tests for metric queries with custom granularity Sep 18, 2024
@courtneyholcomb courtneyholcomb changed the title Write SQL rendering tests for metric queries with custom granularity SQL rendering tests for metric queries with custom granularity Sep 18, 2024
@courtneyholcomb courtneyholcomb added the Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment label Sep 18, 2024
@github-actions github-actions bot removed the Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment label Sep 18, 2024
@courtneyholcomb courtneyholcomb force-pushed the court/custom-grain-dfp-7 branch 2 times, most recently from bd5d6b1 to c6b4526 Compare September 24, 2024 16:49
@courtneyholcomb courtneyholcomb added the Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment label Sep 24, 2024
@github-actions github-actions bot removed the Run Tests With Other SQL Engines Runs the test suite against the SQL engines in our target environment label Sep 24, 2024
Copy link
Contributor

@tlento tlento left a 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.

courtneyholcomb added a commit that referenced this pull request Sep 24, 2024
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.
Base automatically changed from court/custom-grain-dfp-6 to main September 24, 2024 21:08
@courtneyholcomb courtneyholcomb merged commit 727a380 into main Sep 24, 2024
15 checks passed
@courtneyholcomb courtneyholcomb deleted the court/custom-grain-dfp-7 branch September 24, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants