Skip to content

Commit 17b673e

Browse files
committed
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
1 parent 94a2175 commit 17b673e

3 files changed

Lines changed: 316 additions & 66 deletions

File tree

packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5708,4 +5708,50 @@ cubes:
57085708
},
57095709
]);
57105710
});
5711+
5712+
if (getEnv('nativeSqlPlanner')) {
5713+
describe('FILTER_PARAMS with segments under Tesseract', () => {
5714+
const fpCompilers = prepareJsCompiler(`
5715+
cube('orders_fp', {
5716+
sql: \`
5717+
SELECT * FROM orders
5718+
WHERE \${FILTER_PARAMS.orders_fp.created_at.filter('created_at')}
5719+
AND \${FILTER_PARAMS.orders_fp.status.filter('status')}
5720+
\`,
5721+
measures: {
5722+
count: { type: 'count' },
5723+
},
5724+
dimensions: {
5725+
id: { sql: 'id', type: 'number', primaryKey: true },
5726+
status: { sql: 'status', type: 'string' },
5727+
created_at: { sql: 'created_at', type: 'time' },
5728+
},
5729+
segments: {
5730+
completed: { sql: \`\${CUBE}.status = 'completed'\` },
5731+
},
5732+
});
5733+
`);
5734+
5735+
it('FILTER_PARAMS pushdown is preserved when a segment is present', async () => {
5736+
await fpCompilers.compiler.compile();
5737+
const query = new PostgresQuery(fpCompilers, {
5738+
measures: ['orders_fp.count'],
5739+
segments: ['orders_fp.completed'],
5740+
filters: [
5741+
{ member: 'orders_fp.status', operator: 'equals', values: ['completed'] },
5742+
],
5743+
timeDimensions: [{
5744+
dimension: 'orders_fp.created_at',
5745+
dateRange: ['2024-01-01', '2024-01-31'],
5746+
}],
5747+
});
5748+
5749+
const [sql] = query.buildSqlAndParams();
5750+
5751+
expect(sql).not.toMatch(/WHERE\s+1\s*=\s*1\s+AND\s+1\s*=\s*1/);
5752+
expect(sql).toMatch(/created_at/);
5753+
expect(sql).toMatch(/status/);
5754+
});
5755+
});
5756+
}
57115757
});

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

Lines changed: 64 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ use cubenativeutils::CubeError;
66
use std::fmt;
77
use std::rc::Rc;
88

9+
/// Whether a recursive `find_subtree_for_members` call matched every node
10+
/// without pruning anything (segments, non-matching items, etc.).
11+
type FullMatch = bool;
12+
913
#[derive(Clone, PartialEq)]
1014
pub enum FilterGroupOperator {
1115
Or,
@@ -128,88 +132,82 @@ impl FilterItem {
128132
}
129133
}
130134

131-
/// Extract all member symbols from this filter tree
132-
/// Returns None if filter tree is invalid (e.g., empty group)
133-
/// Returns Some(set) with all member symbols found in the tree
134-
fn extract_filter_members(&self) -> Option<Vec<Rc<MemberSymbol>>> {
135-
match self {
136-
FilterItem::Group(group) => {
137-
// Empty groups are considered invalid
138-
if group.items.is_empty() {
139-
return None;
140-
}
141-
142-
let mut all_members = Vec::new();
143-
144-
// Recursively extract from all children
145-
for child in &group.items {
146-
match child.extract_filter_members() {
147-
None => return None, // If any child is invalid, entire tree is invalid
148-
Some(mut members) => all_members.append(&mut members),
149-
}
150-
}
151-
152-
Some(all_members)
153-
}
154-
FilterItem::Item(item) => Some(vec![item.member_evaluator().clone()]),
155-
FilterItem::Segment(_) => None,
156-
}
157-
}
158-
159-
/// Find subtree of filters that only contains filters for the specified members
160-
/// Returns None if no matching filters found
161-
/// Returns Some(FilterItem) with the subtree containing only filters for target members
135+
/// Find subtree of filters that only contains filters for the specified members.
136+
/// Returns `None` if no matching filters found.
137+
/// Returns `Some(FilterItem)` with the subtree containing only filters for target members.
138+
///
139+
/// Partial matching is only supported for AND groups. OR groups only match
140+
/// if all of their children match the target members.
162141
///
163-
/// This only processes AND groups - OR groups are not supported and will return None
142+
/// `Segment` nodes are skipped during extraction -- they do not prevent
143+
/// sibling member filters from being collected in AND groups.
164144
pub fn find_subtree_for_members(&self, target_members: &[&String]) -> Option<FilterItem> {
145+
self.find_subtree_for_members_inner(target_members)
146+
.map(|(filter_item, _)| filter_item)
147+
}
148+
149+
fn find_subtree_for_members_inner(
150+
&self,
151+
target_members: &[&String],
152+
) -> Option<(FilterItem, FullMatch)> {
165153
match self {
166154
FilterItem::Group(group) => {
167155
// Empty groups return None
168156
if group.items.is_empty() {
169157
return None;
170158
}
171159

172-
// Extract all members from this filter subtree
173-
let filter_members = self.extract_filter_members()?;
160+
match group.operator {
161+
FilterGroupOperator::And => {
162+
let mut matching_children = Vec::new();
163+
let mut all_children_fully_match = true;
174164

175-
// Check if all members in this filter are in the target set
176-
let all_members_match = filter_members.iter().all(|member| {
177-
target_members.iter().any(|target| {
178-
&&member.clone().resolve_reference_chain().full_name() == target
179-
})
180-
});
165+
for child in &group.items {
166+
match child.find_subtree_for_members_inner(target_members) {
167+
Some((matching_child, child_fully_matched)) => {
168+
matching_children.push(matching_child);
169+
all_children_fully_match &= child_fully_matched;
170+
}
171+
None => all_children_fully_match = false,
172+
}
173+
}
181174

182-
if all_members_match {
183-
// All members match - return this entire filter subtree
184-
return Some(self.clone());
185-
}
175+
if matching_children.is_empty() {
176+
return None;
177+
}
186178

187-
// Only process AND groups for partial matching
188-
if group.operator == FilterGroupOperator::And {
189-
let matching_children: Vec<FilterItem> = group
190-
.items
191-
.iter()
192-
.filter_map(|child| child.find_subtree_for_members(target_members))
193-
.collect();
179+
if all_children_fully_match {
180+
// Every child matches, so preserve the original group shape.
181+
return Some((self.clone(), true));
182+
}
194183

195-
if matching_children.is_empty() {
196-
return None;
197-
}
184+
if matching_children.len() == 1 {
185+
// Single match - return it directly without wrapping.
186+
return Some((matching_children.into_iter().next().unwrap(), false));
187+
}
198188

199-
if matching_children.len() == 1 {
200-
// Single match - return it directly without wrapping
201-
return Some(matching_children.into_iter().next().unwrap());
189+
// Multiple matches - wrap in a new AND group.
190+
Some((
191+
FilterItem::Group(Rc::new(FilterGroup::new(
192+
FilterGroupOperator::And,
193+
matching_children,
194+
))),
195+
false,
196+
))
202197
}
198+
FilterGroupOperator::Or => {
199+
// OR groups can only be preserved if every child matches.
200+
for child in &group.items {
201+
let (_, child_fully_matched) =
202+
child.find_subtree_for_members_inner(target_members)?;
203+
if !child_fully_matched {
204+
return None;
205+
}
206+
}
203207

204-
// Multiple matches - wrap in new AND group
205-
return Some(FilterItem::Group(Rc::new(FilterGroup::new(
206-
FilterGroupOperator::And,
207-
matching_children,
208-
))));
208+
Some((self.clone(), true))
209+
}
209210
}
210-
211-
// OR groups are not supported
212-
None
213211
}
214212
FilterItem::Item(item) => {
215213
let member = item.member_evaluator();
@@ -219,7 +217,7 @@ impl FilterItem {
219217
.iter()
220218
.any(|target| &&member.clone().resolve_reference_chain().full_name() == target)
221219
{
222-
Some(self.clone())
220+
Some((self.clone(), true))
223221
} else {
224222
None
225223
}

0 commit comments

Comments
 (0)