From 3aaca4b46fdf5986a94a76c5435b5cf6cc3805eb Mon Sep 17 00:00:00 2001 From: tlento Date: Tue, 25 Jun 2024 13:19:20 -0700 Subject: [PATCH] Fix bug with functionally identical comparisons on WhereConstraintNode Due to an oversight the functionally_identical and with_new_parents logic was not updated on the WhereConstraintNode to account for the always_apply property. In isolation this doesn't affect the predicate pushdown optimizer, but in conjunction with the source scan optimizer this causes problems. Rather than merge something broken I'm amending this PR. --- metricflow/dataflow/nodes/where_filter.py | 14 ++++++++++---- .../optimizer/predicate_pushdown_optimizer.py | 7 +++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/metricflow/dataflow/nodes/where_filter.py b/metricflow/dataflow/nodes/where_filter.py index 60dcc9a1f6..04240a9029 100644 --- a/metricflow/dataflow/nodes/where_filter.py +++ b/metricflow/dataflow/nodes/where_filter.py @@ -66,14 +66,20 @@ def description(self) -> str: # noqa: D102 @property def displayed_properties(self) -> Sequence[DisplayedProperty]: # noqa: D102 - return tuple(super().displayed_properties) + (DisplayedProperty("where_condition", self.where),) + properties = tuple(super().displayed_properties) + (DisplayedProperty("where_condition", self.where),) + if self.always_apply: + properties = properties + (DisplayedProperty("All filters always applied:", self.always_apply),) + return properties def functionally_identical(self, other_node: DataflowPlanNode) -> bool: # noqa: D102 - return isinstance(other_node, self.__class__) and other_node.where == self.where + return ( + isinstance(other_node, self.__class__) + and other_node.where == self.where + and other_node.always_apply == self.always_apply + ) def with_new_parents(self, new_parent_nodes: Sequence[DataflowPlanNode]) -> WhereConstraintNode: # noqa: D102 assert len(new_parent_nodes) == 1 return WhereConstraintNode( - parent_node=new_parent_nodes[0], - where_specs=self.input_where_specs, + parent_node=new_parent_nodes[0], where_specs=self.input_where_specs, always_apply=self.always_apply ) diff --git a/metricflow/dataflow/optimizer/predicate_pushdown_optimizer.py b/metricflow/dataflow/optimizer/predicate_pushdown_optimizer.py index 3a9a2fe995..2c1cbf01e2 100644 --- a/metricflow/dataflow/optimizer/predicate_pushdown_optimizer.py +++ b/metricflow/dataflow/optimizer/predicate_pushdown_optimizer.py @@ -340,6 +340,13 @@ def visit_where_constraint_node(self, node: WhereConstraintNode) -> OptimizeBran ) if node.always_apply: + logger.log( + level=self._log_level, + msg=( + "Applying original filter spec set based on node-level override directive. Additional specs " + + f"appled:\n{[spec for spec in node.input_where_specs if spec not in filter_specs_to_apply]}" + ), + ) optimized_node = OptimizeBranchResult( optimized_branch=node.with_new_parents((optimized_parent.optimized_branch,)) )