fix(tesseract): preserve FILTER_PARAMS pushdown with segments

Tesseract was treating any segment inside a filter tree as a fatal subtree extraction failure, which made FILTER_PARAMS fall back to always_true(). Keep AND-group pruning for matching members, require full matches for OR groups, and add regressions for segment siblings and partial OR branches.

Made-with: Cursor

Author: hank-sq

rust/cubesqlplanner/cubesqlplanner/src/plan/filter.rs

@@ -128,88 +128,79 @@ impl FilterItem {
         }
     }
 
-    /// Extract all member symbols from this filter tree
-    /// Returns None if filter tree is invalid (e.g., empty group)
-    /// Returns Some(set) with all member symbols found in the tree
-    fn extract_filter_members(&self) -> Option<Vec<Rc<MemberSymbol>>> {
-        match self {
-            FilterItem::Group(group) => {
-                // Empty groups are considered invalid
-                if group.items.is_empty() {
-                    return None;
-                }
-
-                let mut all_members = Vec::new();
-
-                // Recursively extract from all children
-                for child in &group.items {
-                    match child.extract_filter_members() {
-                        None => return None, // If any child is invalid, entire tree is invalid
-                        Some(mut members) => all_members.append(&mut members),
-                    }
-                }
-
-                Some(all_members)
-            }
-            FilterItem::Item(item) => Some(vec![item.member_evaluator().clone()]),
-            FilterItem::Segment(_) => None,
-        }
-    }
-
     /// Find subtree of filters that only contains filters for the specified members
     /// Returns None if no matching filters found
     /// Returns Some(FilterItem) with the subtree containing only filters for target members
     ///
-    /// This only processes AND groups - OR groups are not supported and will return None
+    /// Partial matching is only supported for AND groups. OR groups only match
+    /// if all of their children match the target members.
     pub fn find_subtree_for_members(&self, target_members: &[&String]) -> Option<FilterItem> {
+        self.find_subtree_for_members_with_match_state(target_members)
+            .map(|(filter_item, _)| filter_item)
+    }
+
+    fn find_subtree_for_members_with_match_state(
+        &self,
+        target_members: &[&String],
+    ) -> Option<(FilterItem, bool)> {
         match self {
             FilterItem::Group(group) => {
                 // Empty groups return None
                 if group.items.is_empty() {
                     return None;
                 }
 
-                // Extract all members from this filter subtree
-                let filter_members = self.extract_filter_members()?;
+                match group.operator {
+                    FilterGroupOperator::And => {
+                        let mut matching_children = Vec::new();
+                        let mut all_children_fully_match = true;
+
+                        for child in &group.items {
+                            match child.find_subtree_for_members_with_match_state(target_members) {
+                                Some((matching_child, child_fully_matched)) => {
+                                    matching_children.push(matching_child);
+                                    all_children_fully_match &= child_fully_matched;
+                                }
+                                None => all_children_fully_match = false,
+                            }
+                        }
 
-                // Check if all members in this filter are in the target set
-                let all_members_match = filter_members.iter().all(|member| {
-                    target_members.iter().any(|target| {
-                        &&member.clone().resolve_reference_chain().full_name() == target
-                    })
-                });
+                        if matching_children.is_empty() {
+                            return None;
+                        }
 
-                if all_members_match {
-                    // All members match - return this entire filter subtree
-                    return Some(self.clone());
-                }
+                        if all_children_fully_match {
+                            // Every child matches, so preserve the original group shape.
+                            return Some((self.clone(), true));
+                        }
 
-                // Only process AND groups for partial matching
-                if group.operator == FilterGroupOperator::And {
-                    let matching_children: Vec<FilterItem> = group
-                        .items
-                        .iter()
-                        .filter_map(|child| child.find_subtree_for_members(target_members))
-                        .collect();
+                        if matching_children.len() == 1 {
+                            // Single match - return it directly without wrapping.
+                            return Some((matching_children.into_iter().next().unwrap(), false));
+                        }
 
-                    if matching_children.is_empty() {
-                        return None;
+                        // Multiple matches - wrap in a new AND group.
+                        Some((
+                            FilterItem::Group(Rc::new(FilterGroup::new(
+                                FilterGroupOperator::And,
+                                matching_children,
+                            ))),
+                            false,
+                        ))
                     }
+                    FilterGroupOperator::Or => {
+                        // OR groups can only be preserved if every child matches.
+                        for child in &group.items {
+                            let (_, child_fully_matched) =
+                                child.find_subtree_for_members_with_match_state(target_members)?;
+                            if !child_fully_matched {
+                                return None;
+                            }
+                        }
 
-                    if matching_children.len() == 1 {
-                        // Single match - return it directly without wrapping
-                        return Some(matching_children.into_iter().next().unwrap());
+                        Some((self.clone(), true))
                     }
-
-                    // Multiple matches - wrap in new AND group
-                    return Some(FilterItem::Group(Rc::new(FilterGroup::new(
-                        FilterGroupOperator::And,
-                        matching_children,
-                    ))));
                 }
-
-                // OR groups are not supported
-                None
             }
             FilterItem::Item(item) => {
                 let member = item.member_evaluator();
@@ -219,7 +210,7 @@ impl FilterItem {
                     .iter()
                     .any(|target| &&member.clone().resolve_reference_chain().full_name() == target)
                 {
-                    Some(self.clone())
+                    Some((self.clone(), true))
                 } else {
                     None
                 }

rust/cubesqlplanner/cubesqlplanner/src/tests/utils/debug.rs

@@ -4,6 +4,7 @@ use crate::planner::filter::{BaseFilter, FilterOperator};
 use crate::planner::sql_evaluator::DebugSql;
 use crate::test_fixtures::cube_bridge::MockSchema;
 use crate::test_fixtures::test_utils::TestContext;
+use std::rc::Rc;
 
 #[test]
 fn test_dimension_basic() {
@@ -201,3 +202,137 @@ fn test_filter_group_and_collapsed() {
         "AND: [\n  {visitors.source} equals: ['google'],\n  {visitors.id} gt: ['100']\n]";
     assert_eq!(sql, expected);
 }
+
+#[test]
+fn test_find_subtree_for_members_excludes_segments_from_and_group() {
+    let schema = MockSchema::from_yaml_file("common/integration_basic.yaml");
+    let ctx = TestContext::new(schema).unwrap();
+    let city_symbol = ctx.create_symbol("customers.city").unwrap();
+    let amount_symbol = ctx.create_symbol("orders.amount").unwrap();
+
+    let city_filter = BaseFilter::try_new(
+        ctx.query_tools().clone(),
+        city_symbol.clone(),
+        crate::planner::filter::base_filter::FilterType::Dimension,
+        FilterOperator::Equal,
+        Some(vec![Some("New York".to_string())]),
+    )
+    .unwrap();
+
+    let amount_filter = BaseFilter::try_new(
+        ctx.query_tools().clone(),
+        amount_symbol.clone(),
+        crate::planner::filter::base_filter::FilterType::Dimension,
+        FilterOperator::Gte,
+        Some(vec![Some("100".to_string())]),
+    )
+    .unwrap();
+
+    let completed_segment = ctx.create_segment("orders.completed_orders").unwrap();
+
+    let filter_tree = FilterItem::Group(Rc::new(FilterGroup::new(
+        FilterGroupOperator::And,
+        vec![
+            FilterItem::Item(city_filter),
+            FilterItem::Item(amount_filter),
+            FilterItem::Segment(completed_segment),
+        ],
+    )));
+
+    let city_member = city_symbol.resolve_reference_chain().full_name();
+    let amount_member = amount_symbol.resolve_reference_chain().full_name();
+    let targets = vec![&city_member, &amount_member];
+
+    let subtree = filter_tree
+        .find_subtree_for_members(&targets)
+        .expect("matching filters should still be extracted");
+
+    assert_eq!(
+        subtree.debug_sql(false),
+        "AND: [\n  {customers.city} equals: ['New York'],\n  {orders.amount} gte: ['100']\n]"
+    );
+}
+
+#[test]
+fn test_find_subtree_for_members_keeps_or_groups_all_or_nothing() {
+    let schema = MockSchema::from_yaml_file("common/integration_basic.yaml");
+    let ctx = TestContext::new(schema).unwrap();
+    let city_symbol = ctx.create_symbol("customers.city").unwrap();
+
+    let city_filter = BaseFilter::try_new(
+        ctx.query_tools().clone(),
+        city_symbol.clone(),
+        crate::planner::filter::base_filter::FilterType::Dimension,
+        FilterOperator::Equal,
+        Some(vec![Some("New York".to_string())]),
+    )
+    .unwrap();
+
+    let completed_segment = ctx.create_segment("orders.completed_orders").unwrap();
+
+    let filter_tree = FilterItem::Group(Rc::new(FilterGroup::new(
+        FilterGroupOperator::Or,
+        vec![
+            FilterItem::Item(city_filter),
+            FilterItem::Segment(completed_segment),
+        ],
+    )));
+
+    let city_member = city_symbol.resolve_reference_chain().full_name();
+    let targets = vec![&city_member];
+
+    assert!(
+        filter_tree.find_subtree_for_members(&targets).is_none(),
+        "partial matches should not be extracted from OR groups"
+    );
+}
+
+#[test]
+fn test_find_subtree_for_members_rejects_or_groups_with_partially_matching_children() {
+    let schema = MockSchema::from_yaml_file("common/integration_basic.yaml");
+    let ctx = TestContext::new(schema).unwrap();
+    let city_symbol = ctx.create_symbol("customers.city").unwrap();
+    let amount_symbol = ctx.create_symbol("orders.amount").unwrap();
+
+    let city_filter = BaseFilter::try_new(
+        ctx.query_tools().clone(),
+        city_symbol.clone(),
+        crate::planner::filter::base_filter::FilterType::Dimension,
+        FilterOperator::Equal,
+        Some(vec![Some("New York".to_string())]),
+    )
+    .unwrap();
+
+    let amount_filter = BaseFilter::try_new(
+        ctx.query_tools().clone(),
+        amount_symbol.clone(),
+        crate::planner::filter::base_filter::FilterType::Dimension,
+        FilterOperator::Gte,
+        Some(vec![Some("100".to_string())]),
+    )
+    .unwrap();
+
+    let completed_segment = ctx.create_segment("orders.completed_orders").unwrap();
+
+    let partially_matching_branch = FilterItem::Group(Rc::new(FilterGroup::new(
+        FilterGroupOperator::And,
+        vec![
+            FilterItem::Item(city_filter),
+            FilterItem::Segment(completed_segment),
+        ],
+    )));
+
+    let filter_tree = FilterItem::Group(Rc::new(FilterGroup::new(
+        FilterGroupOperator::Or,
+        vec![partially_matching_branch, FilterItem::Item(amount_filter)],
+    )));
+
+    let city_member = city_symbol.resolve_reference_chain().full_name();
+    let amount_member = amount_symbol.resolve_reference_chain().full_name();
+    let targets = vec![&city_member, &amount_member];
+
+    assert!(
+        filter_tree.find_subtree_for_members(&targets).is_none(),
+        "OR groups should only match when every branch matches without pruning"
+    );
+}