WIP - multi hop tests #1174
Draft
WIP - multi hop tests #1174
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
1) Use new MetricSubqueryJoinPath; 2) Remove redundant attributes; 3) Always require JOINED and METRIC properties
…bquery join on entity
This PR changed how we use the semantic_model_join_path, so this validation is no longer relevant. Now, the semantic_model_join_path does not include the last entity link, since the last join is not joining to a semantic model.
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.
…ricSubqueryJoinPathElement. The user might need to specify an entity path to get to the metric they want. In that case, we might have multiple entity paths to get to a given metric/entity combo. Store all options.
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.
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.
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.
…y ever accept one 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. |
1 similar comment
|
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. |
courtneyholcomb
force-pushed
the
court/gb-metrics16
branch
from
b3d8316
to
b640ed2
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