Remove join_type param

dbt-labs · Nov 4, 2023 · 4c4f440 · 4c4f440
1 parent b243aad
commit 4c4f440
Show file tree

Hide file tree

Showing 25 changed files with 7 additions and 86 deletions.
diff --git a/metricflow/dataflow/builder/dataflow_plan_builder.py b/metricflow/dataflow/builder/dataflow_plan_builder.py
@@ -213,7 +213,6 @@ def _build_metrics_output_node(
         queried_linkable_specs: LinkableSpecSet,
         where_constraint: Optional[WhereFilterSpec] = None,
         time_range_constraint: Optional[TimeRangeConstraint] = None,
-        combine_metrics_join_type: SqlJoinType = SqlJoinType.FULL_OUTER,
     ) -> BaseOutput:
         """Builds a computed metrics output node.
 
@@ -222,7 +221,6 @@ def _build_metrics_output_node(
             queried_linkable_specs: Dimensions/entities that were queried for.
             where_constraint: Where constraint used to compute the metric.
             time_range_constraint: Time range constraint used to compute the metric.
-            combine_metrics_join_type: The join used when combining the computed metrics.
         """
         output_nodes: List[BaseOutput] = []
         compute_metrics_node: Optional[ComputeMetricsNode] = None
@@ -247,7 +245,6 @@ def _build_metrics_output_node(
                         queried_linkable_specs=queried_linkable_specs,
                         where_constraint=where_constraint,
                         time_range_constraint=time_range_constraint,
-                        combine_metrics_join_type=SqlJoinType.FULL_OUTER,
                     ),
                     metric_specs=[metric_spec],
                 )
@@ -294,10 +291,7 @@ def _build_metrics_output_node(
         if len(output_nodes) == 1:
             return output_nodes[0]
 
-        return CombineMetricsNode(
-            parent_nodes=output_nodes,
-            join_type=combine_metrics_join_type,
-        )
+        return CombineMetricsNode(parent_nodes=output_nodes)
 
     def build_plan_for_distinct_values(self, query_spec: MetricFlowQuerySpec) -> DataflowPlan:
         """Generate a plan that would get the distinct values of a linkable instance.

diff --git a/metricflow/dataflow/dataflow_plan.py b/metricflow/dataflow/dataflow_plan.py
@@ -1225,9 +1225,7 @@ class CombineMetricsNode(ComputedMetricsOutput):
     def __init__(  # noqa: D
         self,
         parent_nodes: Sequence[Union[BaseOutput, ComputedMetricsOutput]],
-        join_type: SqlJoinType = SqlJoinType.FULL_OUTER,
     ) -> None:
-        self._join_type = join_type
         super().__init__(node_id=self.create_unique_id(), parent_nodes=list(parent_nodes))
 
     @classmethod
@@ -1241,31 +1239,12 @@ def accept(self, visitor: DataflowPlanNodeVisitor[VisitorOutputT]) -> VisitorOut
     def description(self) -> str:  # noqa: D
         return "Combine Metrics"
 
-    @property
-    def displayed_properties(self) -> List[DisplayedProperty]:
-        """Prints details about the join types and how the node will behave."""
-        custom_properties = [DisplayedProperty("join type", self.join_type)]
-        if self.join_type is SqlJoinType.FULL_OUTER:
-            custom_properties.append(
-                DisplayedProperty("de-duplication method", "post-join aggregation across all dimensions")
-            )
-
-        return super().displayed_properties + custom_properties
-
-    @property
-    def join_type(self) -> SqlJoinType:
-        """The type of join used for combining metrics."""
-        return self._join_type
-
     def functionally_identical(self, other_node: DataflowPlanNode) -> bool:  # noqa: D
-        return isinstance(other_node, self.__class__) and other_node.join_type == self.join_type
+        return isinstance(other_node, self.__class__)
 
     def with_new_parents(self, new_parent_nodes: Sequence[BaseOutput]) -> CombineMetricsNode:  # noqa: D
         assert len(new_parent_nodes) == 1
-        return CombineMetricsNode(
-            parent_nodes=new_parent_nodes,
-            join_type=self.join_type,
-        )
+        return CombineMetricsNode(parent_nodes=new_parent_nodes)
 
 
 class ConstrainTimeRangeNode(AggregatedMeasuresOutput, BaseOutput):

diff --git a/metricflow/dataflow/optimizer/source_scan/source_scan_optimizer.py b/metricflow/dataflow/optimizer/source_scan/source_scan_optimizer.py
@@ -287,9 +287,7 @@ def visit_combine_metrics_node(self, node: CombineMetricsNode) -> OptimizeBranch
         if len(combined_parent_branches) == 1:
             return OptimizeBranchResult(base_output_node=combined_parent_branches[0])
 
-        return OptimizeBranchResult(
-            base_output_node=CombineMetricsNode(parent_nodes=combined_parent_branches, join_type=node.join_type)
-        )
+        return OptimizeBranchResult(base_output_node=CombineMetricsNode(parent_nodes=combined_parent_branches))
 
     def visit_constrain_time_range_node(self, node: ConstrainTimeRangeNode) -> OptimizeBranchResult:  # noqa: D
         self._log_visit_node_type(node)

diff --git a/metricflow/plan_conversion/dataflow_to_sql.py b/metricflow/plan_conversion/dataflow_to_sql.py
@@ -959,7 +959,7 @@ def visit_combine_metrics_node(self, node: CombineMetricsNode) -> SqlDataSet:
         ), "All parent nodes should have the same set of linkable instances since all values are coalesced."
 
         linkable_spec_set = from_data_set.data_set.instance_set.spec_set.transform(SelectOnlyLinkableSpecs())
-        join_type = SqlJoinType.CROSS_JOIN if len(linkable_spec_set.all_specs) == 0 else node.join_type
+        join_type = SqlJoinType.CROSS_JOIN if len(linkable_spec_set.all_specs) == 0 else SqlJoinType.FULL_OUTER
 
         joins_descriptions: List[SqlJoinDescription] = []
         # TODO: refactor this loop into SqlQueryPlanJoinBuilder
@@ -984,7 +984,7 @@ def visit_combine_metrics_node(self, node: CombineMetricsNode) -> SqlDataSet:
         output_instance_set = InstanceSet.merge([x.data_set.instance_set for x in parent_data_sets])
         output_instance_set = output_instance_set.transform(ChangeAssociatedColumns(self._column_association_resolver))
 
-        metric_aggregation_type = AggregationType.MAX if node.join_type is SqlJoinType.FULL_OUTER else None
+        metric_aggregation_type = AggregationType.MAX
         metric_select_column_set = SelectColumnSet(
             metric_columns=self._make_select_columns_for_metrics(
                 table_alias_to_metric_specs, aggregation_type=metric_aggregation_type
@@ -1006,7 +1006,7 @@ def visit_combine_metrics_node(self, node: CombineMetricsNode) -> SqlDataSet:
                 from_source=from_data_set.data_set.sql_select_node,
                 from_source_alias=from_data_set.alias,
                 joins_descs=tuple(joins_descriptions),
-                group_bys=linkable_select_column_set.as_tuple() if node.join_type is SqlJoinType.FULL_OUTER else (),
+                group_bys=linkable_select_column_set.as_tuple(),
                 where=None,
                 order_bys=(),
             ),

diff --git a/...napshots/test_dataflow_plan_builder.py/DataflowPlan/test_common_semantic_model__dfp_0.xml b/...napshots/test_dataflow_plan_builder.py/DataflowPlan/test_common_semantic_model__dfp_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_0 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_0 -->

diff --git a/...test_dataflow_plan_builder.py/DataflowPlan/test_derived_metric_offset_to_grain__dfp_0.xml b/...test_dataflow_plan_builder.py/DataflowPlan/test_derived_metric_offset_to_grain__dfp_0.xml
@@ -15,8 +15,6 @@
             <CombineMetricsNode>
                 <!-- description = Combine Metrics -->
                 <!-- node_id = cbm_0 -->
-                <!-- join type = SqlJoinType.FULL_OUTER -->
-                <!-- de-duplication method = post-join aggregation across all dimensions -->
                 <ComputeMetricsNode>
                     <!-- description = Compute Metrics via Expressions -->
                     <!-- node_id = cm_0 -->

diff --git a/...t_dataflow_plan_builder.py/DataflowPlan/test_join_to_time_spine_derived_metric__dfp_0.xml b/...t_dataflow_plan_builder.py/DataflowPlan/test_join_to_time_spine_derived_metric__dfp_0.xml
@@ -15,8 +15,6 @@
             <CombineMetricsNode>
                 <!-- description = Combine Metrics -->
                 <!-- node_id = cbm_0 -->
-                <!-- join type = SqlJoinType.FULL_OUTER -->
-                <!-- de-duplication method = post-join aggregation across all dimensions -->
                 <ComputeMetricsNode>
                     <!-- description = Compute Metrics via Expressions -->
                     <!-- node_id = cm_0 -->

diff --git a/...pshots/test_dataflow_plan_builder.py/DataflowPlan/test_measure_constraint_plan__dfp_0.xml b/...pshots/test_dataflow_plan_builder.py/DataflowPlan/test_measure_constraint_plan__dfp_0.xml
@@ -15,8 +15,6 @@
             <CombineMetricsNode>
                 <!-- description = Combine Metrics -->
                 <!-- node_id = cbm_0 -->
-                <!-- join type = SqlJoinType.FULL_OUTER -->
-                <!-- de-duplication method = post-join aggregation across all dimensions -->
                 <ComputeMetricsNode>
                     <!-- description = Compute Metrics via Expressions -->
                     <!-- node_id = cm_0 -->

diff --git a/..._plan_builder.py/DataflowPlan/test_measure_constraint_with_reused_measure_plan__dfp_0.xml b/..._plan_builder.py/DataflowPlan/test_measure_constraint_with_reused_measure_plan__dfp_0.xml
@@ -15,8 +15,6 @@
             <CombineMetricsNode>
                 <!-- description = Combine Metrics -->
                 <!-- node_id = cbm_0 -->
-                <!-- join type = SqlJoinType.FULL_OUTER -->
-                <!-- de-duplication method = post-join aggregation across all dimensions -->
                 <ComputeMetricsNode>
                     <!-- description = Compute Metrics via Expressions -->
                     <!-- node_id = cm_0 -->

diff --git a/...flow_plan_builder.py/DataflowPlan/test_multi_semantic_model_ratio_metrics_plan__dfp_0.xml b/...flow_plan_builder.py/DataflowPlan/test_multi_semantic_model_ratio_metrics_plan__dfp_0.xml
@@ -15,8 +15,6 @@
             <CombineMetricsNode>
                 <!-- description = Combine Metrics -->
                 <!-- node_id = cbm_0 -->
-                <!-- join type = SqlJoinType.FULL_OUTER -->
-                <!-- de-duplication method = post-join aggregation across all dimensions -->
                 <ComputeMetricsNode>
                     <!-- description = Compute Metrics via Expressions -->
                     <!-- node_id = cm_0 -->

diff --git a/...napshots/test_dataflow_plan_builder.py/DataflowPlan/test_multiple_metrics_plan__dfp_0.xml b/...napshots/test_dataflow_plan_builder.py/DataflowPlan/test_multiple_metrics_plan__dfp_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_0 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_0 -->

diff --git a/...low_plan_builder.py/DataflowPlan/test_single_semantic_model_ratio_metrics_plan__dfp_0.xml b/...low_plan_builder.py/DataflowPlan/test_single_semantic_model_ratio_metrics_plan__dfp_0.xml
@@ -15,8 +15,6 @@
             <CombineMetricsNode>
                 <!-- description = Combine Metrics -->
                 <!-- node_id = cbm_0 -->
-                <!-- join type = SqlJoinType.FULL_OUTER -->
-                <!-- de-duplication method = post-join aggregation across all dimensions -->
                 <ComputeMetricsNode>
                     <!-- description = Compute Metrics via Expressions -->
                     <!-- node_id = cm_0 -->

diff --git a/...est_source_scan_optimizer.py/DataflowPlan/test_2_metrics_from_1_semantic_model__dfp_0.xml b/...est_source_scan_optimizer.py/DataflowPlan/test_2_metrics_from_1_semantic_model__dfp_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_0 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_0 -->

diff --git a/...st_source_scan_optimizer.py/DataflowPlan/test_2_metrics_from_2_semantic_models__dfp_0.xml b/...st_source_scan_optimizer.py/DataflowPlan/test_2_metrics_from_2_semantic_models__dfp_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_0 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_0 -->

diff --git a/...t_source_scan_optimizer.py/DataflowPlan/test_2_metrics_from_2_semantic_models__dfpo_0.xml b/...t_source_scan_optimizer.py/DataflowPlan/test_2_metrics_from_2_semantic_models__dfpo_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_1 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_2 -->

diff --git a/...urce_scan_optimizer.py/DataflowPlan/test_2_ratio_metrics_from_1_semantic_model__dfp_0.xml b/...urce_scan_optimizer.py/DataflowPlan/test_2_ratio_metrics_from_1_semantic_model__dfp_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_2 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_2 -->
@@ -20,8 +18,6 @@
                 <CombineMetricsNode>
                     <!-- description = Combine Metrics -->
                     <!-- node_id = cbm_0 -->
-                    <!-- join type = SqlJoinType.FULL_OUTER -->
-                    <!-- de-duplication method = post-join aggregation across all dimensions -->
                     <ComputeMetricsNode>
                         <!-- description = Compute Metrics via Expressions -->
                         <!-- node_id = cm_0 -->
@@ -127,8 +123,6 @@
                 <CombineMetricsNode>
                     <!-- description = Combine Metrics -->
                     <!-- node_id = cbm_1 -->
-                    <!-- join type = SqlJoinType.FULL_OUTER -->
-                    <!-- de-duplication method = post-join aggregation across all dimensions -->
                     <ComputeMetricsNode>
                         <!-- description = Compute Metrics via Expressions -->
                         <!-- node_id = cm_3 -->

diff --git a/...st_source_scan_optimizer.py/DataflowPlan/test_3_metrics_from_2_semantic_models__dfp_0.xml b/...st_source_scan_optimizer.py/DataflowPlan/test_3_metrics_from_2_semantic_models__dfp_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_0 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_0 -->

diff --git a/...t_source_scan_optimizer.py/DataflowPlan/test_3_metrics_from_2_semantic_models__dfpo_0.xml b/...t_source_scan_optimizer.py/DataflowPlan/test_3_metrics_from_2_semantic_models__dfpo_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_1 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_6 -->

diff --git a/...est_source_scan_optimizer.py/DataflowPlan/test_constrained_metric_not_combined__dfp_0.xml b/...est_source_scan_optimizer.py/DataflowPlan/test_constrained_metric_not_combined__dfp_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_0 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_0 -->

diff --git a/...st_source_scan_optimizer.py/DataflowPlan/test_constrained_metric_not_combined__dfpo_0.xml b/...st_source_scan_optimizer.py/DataflowPlan/test_constrained_metric_not_combined__dfpo_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_1 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_2 -->

diff --git a/.../test/snapshots/test_source_scan_optimizer.py/DataflowPlan/test_derived_metric__dfp_0.xml b/.../test/snapshots/test_source_scan_optimizer.py/DataflowPlan/test_derived_metric__dfp_0.xml
@@ -15,8 +15,6 @@
             <CombineMetricsNode>
                 <!-- description = Combine Metrics -->
                 <!-- node_id = cbm_0 -->
-                <!-- join type = SqlJoinType.FULL_OUTER -->
-                <!-- de-duplication method = post-join aggregation across all dimensions -->
                 <ComputeMetricsNode>
                     <!-- description = Compute Metrics via Expressions -->
                     <!-- node_id = cm_0 -->

diff --git a/...rce_scan_optimizer.py/DataflowPlan/test_derived_metric_with_non_derived_metric__dfp_0.xml b/...rce_scan_optimizer.py/DataflowPlan/test_derived_metric_with_non_derived_metric__dfp_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_1 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_0 -->
@@ -65,8 +63,6 @@
                 <CombineMetricsNode>
                     <!-- description = Combine Metrics -->
                     <!-- node_id = cbm_0 -->
-                    <!-- join type = SqlJoinType.FULL_OUTER -->
-                    <!-- de-duplication method = post-join aggregation across all dimensions -->
                     <ComputeMetricsNode>
                         <!-- description = Compute Metrics via Expressions -->
                         <!-- node_id = cm_1 -->

diff --git a/...ce_scan_optimizer.py/DataflowPlan/test_derived_metric_with_non_derived_metric__dfpo_0.xml b/...ce_scan_optimizer.py/DataflowPlan/test_derived_metric_with_non_derived_metric__dfpo_0.xml
@@ -5,8 +5,6 @@
         <CombineMetricsNode>
             <!-- description = Combine Metrics -->
             <!-- node_id = cbm_2 -->
-            <!-- join type = SqlJoinType.FULL_OUTER -->
-            <!-- de-duplication method = post-join aggregation across all dimensions -->
             <ComputeMetricsNode>
                 <!-- description = Compute Metrics via Expressions -->
                 <!-- node_id = cm_4 -->

diff --git a/...napshots/test_source_scan_optimizer.py/DataflowPlan/test_nested_derived_metric__dfp_0.xml b/...napshots/test_source_scan_optimizer.py/DataflowPlan/test_nested_derived_metric__dfp_0.xml
@@ -15,8 +15,6 @@
             <CombineMetricsNode>
                 <!-- description = Combine Metrics -->
                 <!-- node_id = cbm_1 -->
-                <!-- join type = SqlJoinType.FULL_OUTER -->
-                <!-- de-duplication method = post-join aggregation across all dimensions -->
                 <ComputeMetricsNode>
                     <!-- description = Compute Metrics via Expressions -->
                     <!-- node_id = cm_2 -->
@@ -30,8 +28,6 @@
                     <CombineMetricsNode>
                         <!-- description = Combine Metrics -->
                         <!-- node_id = cbm_0 -->
-                        <!-- join type = SqlJoinType.FULL_OUTER -->
-                        <!-- de-duplication method = post-join aggregation across all dimensions -->
                         <ComputeMetricsNode>
                             <!-- description = Compute Metrics via Expressions -->
                             <!-- node_id = cm_0 -->

diff --git a/...apshots/test_source_scan_optimizer.py/DataflowPlan/test_nested_derived_metric__dfpo_0.xml b/...apshots/test_source_scan_optimizer.py/DataflowPlan/test_nested_derived_metric__dfpo_0.xml
@@ -15,8 +15,6 @@
             <CombineMetricsNode>
                 <!-- description = Combine Metrics -->
                 <!-- node_id = cbm_2 -->
-                <!-- join type = SqlJoinType.FULL_OUTER -->
-                <!-- de-duplication method = post-join aggregation across all dimensions -->
                 <ComputeMetricsNode>
                     <!-- description = Compute Metrics via Expressions -->
                     <!-- node_id = cm_9 -->