PR feedback

dbt-labs · Dec 11, 2024 · 9be730c · 9be730c
1 parent 5770e36
commit 9be730c
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 11 deletions.
diff --git a/metricflow/dataflow/builder/dataflow_plan_builder.py b/metricflow/dataflow/builder/dataflow_plan_builder.py
@@ -80,7 +80,7 @@
 )
 from metricflow.dataflow.nodes.add_generated_uuid import AddGeneratedUuidColumnNode
 from metricflow.dataflow.nodes.aggregate_measures import AggregateMeasuresNode
-from metricflow.dataflow.nodes.alias_specs import AliasSpecsNode
+from metricflow.dataflow.nodes.alias_specs import AliasSpecsNode, SpecToAlias
 from metricflow.dataflow.nodes.combine_aggregated_outputs import CombineAggregatedOutputsNode
 from metricflow.dataflow.nodes.compute_metrics import ComputeMetricsNode
 from metricflow.dataflow.nodes.constrain_time import ConstrainTimeRangeNode
@@ -1880,9 +1880,9 @@ def _build_time_spine_node(
         time_spine_node = AliasSpecsNode.create(
             parent_node=read_node,
             change_specs=tuple(
-                (
-                    time_spine_data_set.instance_from_time_dimension_grain_and_date_part(required_spec).spec,
-                    required_spec,
+                SpecToAlias(
+                    input_spec=time_spine_data_set.instance_from_time_dimension_grain_and_date_part(required_spec).spec,
+                    output_spec=required_spec,
                 )
                 for required_spec in required_time_spine_specs
             ),

diff --git a/metricflow/dataflow/nodes/alias_specs.py b/metricflow/dataflow/nodes/alias_specs.py
@@ -13,20 +13,26 @@
 from metricflow.dataflow.dataflow_plan_visitor import DataflowPlanNodeVisitor
 
 
+@dataclass
+class SpecToAlias:
+    """A mapping of an input spec that should be aliased to match an output spec."""
+
+    input_spec: InstanceSpec
+    output_spec: InstanceSpec
+
+
 @dataclass(frozen=True, eq=False)
 class AliasSpecsNode(DataflowPlanNode, ABC):
     """Change the columns matching the key specs to match the value specs."""
 
-    change_specs: Sequence[Tuple[InstanceSpec, InstanceSpec]]
+    change_specs: Tuple[SpecToAlias, ...]
 
     def __post_init__(self) -> None:  # noqa: D105
         super().__post_init__()
         assert len(self.change_specs) > 0, "Must have at least one value in change_specs for AliasSpecsNode."
 
     @staticmethod
-    def create(  # noqa: D102
-        parent_node: DataflowPlanNode, change_specs: Sequence[Tuple[InstanceSpec, InstanceSpec]]
-    ) -> AliasSpecsNode:
+    def create(parent_node: DataflowPlanNode, change_specs: Tuple[SpecToAlias, ...]) -> AliasSpecsNode:  # noqa: D102
         return AliasSpecsNode(parent_nodes=(parent_node,), change_specs=change_specs)
 
     @classmethod

diff --git a/metricflow/dataset/sql_dataset.py b/metricflow/dataset/sql_dataset.py
@@ -145,7 +145,7 @@ def instances_for_time_dimensions(
         return instances_to_return
 
     def instance_for_time_dimension(self, time_dimension_spec: TimeDimensionSpec) -> TimeDimensionInstance:
-        """Given ta time dimension spec, return the instance associated with it in the data set."""
+        """Given a time dimension spec, return the instance associated with it in the data set."""
         instances = self.instances_for_time_dimensions((time_dimension_spec,))
         if not len(instances) == 1:
             raise RuntimeError(
@@ -160,7 +160,9 @@ def instance_for_spec(self, spec: InstanceSpec) -> MdoInstance:
         for instance in instances:
             if instance.spec == spec:
                 return instance
-        raise RuntimeError(f"Did not find instance matching spec in dataset. Spec: {spec}\nInstances: {instances}")
+        raise RuntimeError(
+            str(LazyFormat("Did not find instance matching spec in dataset.", spec=spec, instances=instances))
+        )
 
     def instance_from_time_dimension_grain_and_date_part(
         self, time_dimension_spec: TimeDimensionSpec

diff --git a/metricflow/plan_conversion/dataflow_to_sql.py b/metricflow/plan_conversion/dataflow_to_sql.py
@@ -1471,7 +1471,10 @@ def visit_alias_specs_node(self, node: AliasSpecsNode) -> SqlDataSet:  # noqa: D
         new_instances: Tuple[MdoInstance, ...] = ()
         new_select_columns: Tuple[SqlSelectColumn, ...] = ()
         instances_to_remove_from_parent: Set[MdoInstance] = set()
-        for old_spec, new_spec in node.change_specs:
+        for spec_to_alias in node.change_specs:
+            old_spec = spec_to_alias.input_spec
+            new_spec = spec_to_alias.output_spec
+
             # Find the instance in the parent data set with matching grain & date part.
             old_instance = parent_data_set.instance_for_spec(old_spec)