Build DataflowPlan for custom offset window #1584
Open
+699
−618
Conversation
This file contains 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
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
from
2719954 to
2dcf124
Compare
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
from
db647f3 to
e91fd30
Compare
courtneyholcomb
force-pushed
the
court/custom-offset5
branch
from
57ba604 to
71cef04
Compare
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
from
e91fd30 to
7e13cf3
Compare
courtneyholcomb
force-pushed
the
court/custom-offset5
branch
2 times, most recently
from
6e642e4 to
a9dc916
Compare
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
2 times, most recently
from
d0c473a to
bebe99d
Compare
courtneyholcomb
force-pushed
the
court/custom-offset5
branch
from
a9dc916 to
7de2bd3
Compare
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
2 times, most recently
from
3740d81 to
ef1dbdc
Compare
plypaul
approved these changes
Dec 19, 2024
| def _offset_window_is_custom(self, offset_window: Optional[MetricTimeWindow]) -> bool: | ||
| return ( | ||
| offset_window is not None | ||
| and offset_window.granularity in self._semantic_model_lookup.custom_granularity_names |
There was a problem hiding this comment.
What happens when the granularity is not in custom_granularity_names? Is this handled through a validation?
There was a problem hiding this comment.
In that case it would be a standard granularity! If neither were true, it would raise an error in validations.
| output_node = JoinToTimeSpineNode.create( | ||
| metric_source_node=output_node, | ||
| time_spine_node=time_spine_node, | ||
| requested_agg_time_dimension_specs=queried_agg_time_dimension_specs, | ||
| join_on_time_dimension_spec=self._sort_by_base_granularity(queried_agg_time_dimension_specs)[0], | ||
| offset_window=metric_spec.offset_window, | ||
| offset_window=( |
There was a problem hiding this comment.
Are there cases where you can use a JoinToTimeSpineNode with a custom offset window? If not, an assertion in the class would help.
There was a problem hiding this comment.
Sure, I'll add an assertion to the the __post_init__ for JoinToTimeSpineNode!
courtneyholcomb
force-pushed
the
court/custom-offset5
branch
3 times, most recently
from
a8cd2c0 to
69f253a
Compare
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
from
ef1dbdc to
60da72b
Compare
courtneyholcomb
force-pushed
the
court/custom-offset5
branch
from
69f253a to
1d0dcca
Compare
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
from
e073215 to
74b183b
Compare
courtneyholcomb
force-pushed
the
court/custom-offset5
branch
from
1d0dcca to
53f6507
Compare
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
from
74b183b to
7232d72
Compare
courtneyholcomb
force-pushed
the
court/custom-offset5
branch
3 times, most recently
from
c77b7bb to
2c32711
Compare
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
from
7232d72 to
70e5b61
Compare
courtneyholcomb
force-pushed
the
court/custom-offset5
branch
from
2c32711 to
54edeb1
Compare
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
from
70e5b61 to
4292849
Compare
courtneyholcomb
force-pushed
the
court/custom-offset5
branch
from
cd61ca8 to
e3af638
Compare
Note that we will also implement a simpler dataflow plan if the user queries the metric with only the granularity used in the offset window. This might not be done before the initial feature release, but the results should be the same. Only differences in performance and readability.
Should only have changes to column order. Only unoptimized queries are impacted.
courtneyholcomb
force-pushed
the
court/custom-offset6
branch
from
4292849 to
e8a3d58
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is the dataflow plan that will be used if the custom grain is queried with any grains that aren't the same as the grain used in the offset window. There will be a simpler dataflow plan used for queries using only the same grain as what's used in the offset window.