-
Notifications
You must be signed in to change notification settings - Fork 96
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
Court/gb reso metric2 #1153
Closed
Closed
Court/gb reso metric2 #1153
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
…specific group by resolution
…create_linkable_element_set_from_join_path
Needed to resolve the appropriate join path for multi-hop joins. Moved source node-building logic to query parser class to avoid cirular import errors. This change also required adding the query parser as a dependency for the dataflow plan builder.
…in_path LinkableMetrics are extra complicated because they involve multiple join paths: 1) the join path needed to join the metric subquery to the outer query, and 2) the join path used in the subquery to join the metric to the group by entity. So far we have only been tracking join path 1. This commit adds tracking for join path 2.
Needed when building source node for group by metric.
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
…y ever accept one group by metric
courtneyholcomb
force-pushed
the
court/gb-reso-metric2
branch
from
April 25, 2024 22:47
b770704
to
229b798
Compare
Originally I left out this last entity link because it was duplicated in the outer query entity links. Turns out this is needed when there are no entity links left in the outer query, because otherwise we won't know what entity the metric is being grouped by.
courtneyholcomb
force-pushed
the
court/gb-reso-metric2
branch
2 times, most recently
from
April 26, 2024 18:44
f640e2c
to
3d945ca
Compare
Originally I thought this wasn't necessary since the entity links should be a duplicate of the entities in the join path. Later I reailzed that there is one scenario where they don't match the join path. We allow querying local entities both alone and prefixed with any of the other entities in the same semantic model. If you query a local entity with another entity prefix, the entity links won't match the join path. To account for that, we need to store both entity links and join path on MetricSubqueryJoinPathElement.
courtneyholcomb
force-pushed
the
court/gb-reso-metric2
branch
from
April 29, 2024 19:21
3d945ca
to
6747f52
Compare
There might be multiple linkable metrics available for a given metric_name + entity_links combo. This is because there might be multiple options for entity paths in the metric subquery, which is not accounted for here. Using the qualified name ensures we will have a unique ElementPathKey for each LinkableMetric.
courtneyholcomb
force-pushed
the
court/gb-reso-metric2
branch
from
April 29, 2024 19:25
6747f52
to
e2b616a
Compare
courtneyholcomb
force-pushed
the
court/gb-reso-metric
branch
from
April 30, 2024 00:09
468a7d9
to
a5e405e
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.
Resolves #
Description