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

Court/gb reso metric2 #1153

Closed
wants to merge 37 commits into from

Conversation

courtneyholcomb
Copy link
Contributor

Resolves #

Description

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.
@cla-bot cla-bot bot added the cla:yes label Apr 25, 2024
Copy link

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.

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 courtneyholcomb force-pushed the court/gb-reso-metric2 branch 2 times, most recently from f640e2c to 3d945ca Compare April 26, 2024 18:44
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant