Skip to content

Commit

Permalink
Improve branch combining logic for AggregateMeasuresNode (#1375)
Browse files Browse the repository at this point in the history
  • Loading branch information
courtneyholcomb authored Aug 22, 2024
1 parent 2047c2c commit ad63f5b
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 10 deletions.
7 changes: 7 additions & 0 deletions .changes/unreleased/Features-20240821-112601.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
kind: Features
body: Support combining AggregateMeasuresNodes where metric input measures have aliases,
so long as there are no duplicates.
time: 2024-08-21T11:26:01.714888-07:00
custom:
Author: courtneyholcomb
Issue: "1375"
22 changes: 12 additions & 10 deletions metricflow/dataflow/optimizer/source_scan/cm_branch_combiner.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,25 +239,27 @@ def visit_aggregate_measures_node( # noqa: D102

assert len(combined_parent_nodes) == 1
combined_parent_node = combined_parent_nodes[0]
assert combined_parent_node is not None

combined_metric_input_measure_specs = tuple(
dict.fromkeys(
self._current_left_node.metric_input_measure_specs + current_right_node.metric_input_measure_specs
).keys()
)

# Avoid combining branches if the AggregateMeasuresNode specifies a metric with an alias to avoid
# collisions e.g. two metrics use the same alias for two different measures. This is not always the case,
# so this could be improved later.
seen_aliases = set()
for spec in combined_metric_input_measure_specs:
# Avoid combining branches if the AggregateMeasuresNode specifies a metric with an alias to avoid
# collisions e.g. two metrics use the same alias for two different measures. This is not always the case,
# so this could be improved later.
if spec.alias is not None:
self._log_combine_failure(
left_node=self._current_left_node,
right_node=current_right_node,
combine_failure_reason=f"Metric input measure spec {spec} has an alias",
)
return ComputeMetricsBranchCombinerResult()
if spec.alias in seen_aliases:
self._log_combine_failure(
left_node=self._current_left_node,
right_node=current_right_node,
combine_failure_reason=f"Found multiple metric input measure specs with alias '{spec.alias}'",
)
return ComputeMetricsBranchCombinerResult()
seen_aliases.add(spec.alias)

combined_node = AggregateMeasuresNode.create(
parent_node=combined_parent_node,
Expand Down

0 comments on commit ad63f5b

Please sign in to comment.