Skip to content

Commit f0569ad

Browse files
ovrovr
authored
fix(tesseract): Render dimension mask filters on grouped masked measures (cube-js#11177)
1 parent 0c8cd24 commit f0569ad

9 files changed

Lines changed: 237 additions & 27 deletions

File tree

‎packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class OracleFilter extends BaseFilter {
2626
public likeIgnoreCase(column, not, param, type) {
2727
const p = (!type || type === 'contains' || type === 'ends') ? '\'%\' || ' : '';
2828
const s = (!type || type === 'contains' || type === 'starts') ? ' || \'%\'' : '';
29-
return `${column}${not ? ' NOT' : ''} LIKE ${p}${this.allocateParam(param)}${s}`;
29+
return `${column}${not ? ' NOT' : ''} LIKE ${p}${this.allocateParam(param)}${s} ESCAPE '\\'`;
3030
}
3131
}
3232

@@ -249,8 +249,9 @@ export class OracleQuery extends BaseQuery {
249249
'{% endfor %}' +
250250
') dates';
251251

252+
templates.expressions.like = '{{ expr }} {% if negated %}NOT {% endif %}LIKE {{ pattern }}{% if default_escape %} ESCAPE \'\\\'{% endif %}';
252253
delete templates.expressions.ilike;
253-
templates.tesseract.ilike = 'LOWER({{ expr }}) {% if negated %}NOT {% endif %}LIKE LOWER({{ pattern }})';
254+
templates.tesseract.ilike = 'LOWER({{ expr }}) {% if negated %}NOT {% endif %}LIKE LOWER({{ pattern }}){% if default_escape %} ESCAPE \'\\\'{% endif %}';
254255

255256
// Oracle has no `STRING` type (used by the default in CAST(... AS STRING),
256257
// e.g. the multi-column count() concatenation). CAST to VARCHAR2 requires a

‎packages/cubejs-schema-compiler/test/unit/yaml-schema.test.ts‎

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2192,7 +2192,8 @@ cubes:
21922192
const [sql] = query.buildSqlAndParams();
21932193
expect(sql).not.toMatch(/CASE\s+WHEN/);
21942194
expect(sql).not.toMatch(/owner_id\s*=/);
2195-
expect(sql).toMatch(/-1\s+"transactions__total_cost"/);
2195+
// The Tesseract planner parenthesizes the masked literal, e.g. `(-1)`.
2196+
expect(sql).toMatch(/\(?-1\)?\s+"transactions__total_cost"/);
21962197
});
21972198

21982199
it('still applies conditional CASE WHEN masking for aggregate measures when the filter member is in the group by', async () => {
@@ -2236,6 +2237,48 @@ cubes:
22362237
expect(sql).toMatch(/owner_id/);
22372238
});
22382239

2240+
it('tesseract: still applies conditional CASE WHEN masking for aggregate measures when the filter member is in the group by', async () => {
2241+
const compilers = prepareYamlCompiler(`
2242+
cubes:
2243+
- name: transactions
2244+
sql_table: public.transactions
2245+
dimensions:
2246+
- name: id
2247+
sql: id
2248+
type: number
2249+
primary_key: true
2250+
- name: org_id
2251+
sql: org_id
2252+
type: string
2253+
- name: owner_id
2254+
sql: owner_id
2255+
type: string
2256+
measures:
2257+
- name: total_cost
2258+
sql: cost
2259+
type: sum
2260+
`);
2261+
2262+
await compilers.compiler.compile();
2263+
2264+
const query = new PostgresQuery(compilers, {
2265+
measures: ['transactions.total_cost'],
2266+
dimensions: ['transactions.org_id', 'transactions.owner_id'],
2267+
maskedMembers: [{
2268+
member: 'transactions.total_cost',
2269+
filter: {
2270+
member: 'transactions.owner_id',
2271+
operator: 'equals',
2272+
values: ['42'],
2273+
}
2274+
}],
2275+
useNativeSqlPlanner: true,
2276+
});
2277+
const [sql] = query.buildSqlAndParams();
2278+
expect(sql).toMatch(/CASE\s+WHEN/);
2279+
expect(sql).toMatch(/owner_id/);
2280+
});
2281+
22392282
it('does not recurse when filter member is also masked', async () => {
22402283
const compilers = prepareYamlCompiler(`
22412284
cubes:
@@ -2279,6 +2322,53 @@ cubes:
22792322
expect(sql).toMatch(/product_id/);
22802323
expect(sql).not.toMatch(/Maximum call stack/);
22812324
});
2325+
2326+
it('tesseract: does not recurse when filter member is also masked', async () => {
2327+
const compilers = prepareYamlCompiler(`
2328+
cubes:
2329+
- name: items
2330+
sql_table: public.items
2331+
dimensions:
2332+
- name: id
2333+
sql: id
2334+
type: number
2335+
primary_key: true
2336+
- name: product_id
2337+
sql: product_id
2338+
type: number
2339+
- name: price
2340+
sql: price
2341+
type: number
2342+
mask: -1
2343+
measures:
2344+
- name: count
2345+
type: count
2346+
`);
2347+
2348+
await compilers.compiler.compile();
2349+
2350+
const query = new PostgresQuery(compilers, {
2351+
measures: ['items.count'],
2352+
dimensions: ['items.product_id', 'items.price'],
2353+
maskedMembers: [
2354+
{
2355+
member: 'items.product_id',
2356+
filter: { member: 'items.product_id', operator: 'lte', values: ['3'] }
2357+
},
2358+
{
2359+
member: 'items.price',
2360+
filter: { member: 'items.product_id', operator: 'lte', values: ['3'] }
2361+
},
2362+
],
2363+
useNativeSqlPlanner: true,
2364+
});
2365+
// The filter member (product_id) is itself masked; rendering the mask filter
2366+
// through the "skip"-masking unmasked tree must not recurse back into masking.
2367+
const [sql] = query.buildSqlAndParams();
2368+
expect(sql).toMatch(/CASE\s+WHEN/);
2369+
expect(sql).toMatch(/product_id/);
2370+
expect(sql).not.toMatch(/Maximum call stack/);
2371+
});
22822372
});
22832373

22842374
describe('Multi-stage filter directive', () => {

‎packages/cubejs-testing-drivers/test/__snapshots__/oracle-full.test.ts.snap‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8670,7 +8670,13 @@ Array [
86708670

86718671
exports[`Queries with the @cubejs-backend/oracle-driver filtering Products: contains + dimensions + order, third 1`] = `Array []`;
86728672

8673-
exports[`Queries with the @cubejs-backend/oracle-driver filtering Products: contains with special chars + dimensions 1`] = `Array []`;
8673+
exports[`Queries with the @cubejs-backend/oracle-driver filtering Products: contains with special chars + dimensions 1`] = `
8674+
Array [
8675+
Object {
8676+
"Products.productName": "Logitech di_Novo Edge Keyboard",
8677+
},
8678+
]
8679+
`;
86748680

86758681
exports[`Queries with the @cubejs-backend/oracle-driver filtering Products: endsWith filter + dimensions + order, first 1`] = `
86768682
Array [

‎rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_nodes/factory.rs‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::physical_plan::cube_ref_evaluator::CubeRefEvaluator;
1010
use crate::physical_plan::sql_nodes::calendar_time_shift::CalendarTimeShiftSqlNode;
1111
use crate::physical_plan::sql_nodes::RenderReferences;
1212
use crate::planner::planners::multi_stage::TimeShiftState;
13+
use crate::planner::query_tools::QueryTools;
1314
use crate::planner::symbols::CalendarDimensionTimeShift;
1415
use std::collections::{HashMap, HashSet};
1516
use std::rc::Rc;
@@ -170,9 +171,34 @@ impl SqlNodesFactory {
170171
/// multi-stage wraps → mask). The whole tree is then wrapped in
171172
/// a top-level `RenderReferencesSqlNode` for query-wide reference
172173
/// substitution.
173-
pub fn default_node_processor(&self) -> Rc<dyn SqlNode> {
174-
let evaluate_sql_processor =
175-
MaskedSqlNode::new(EvaluateSqlNode::new(), self.group_by_members.clone());
174+
pub fn default_node_processor(&self, query_tools: &QueryTools) -> Rc<dyn SqlNode> {
175+
// Build an "unmasked" copy of the tree (masking disabled, but still
176+
// dispatching by member kind) only when the query has masked members. It
177+
// is handed to the masked nodes so they can render mask-filter member
178+
// references through it — routing dimensions through the dimension chain
179+
// and avoiding mask recursion. For queries with no masked members
180+
// `MaskedSqlNode::resolve_mask` short-circuits and never dereferences the
181+
// unmasked root, so building it would be pure waste; passing `None` falls
182+
// back to the masked node's own input (see masked.rs).
183+
let unmasked_root = if query_tools.has_masked_members() {
184+
Some(self.build_node_processor(true, None))
185+
} else {
186+
None
187+
};
188+
self.build_node_processor(false, unmasked_root)
189+
}
190+
191+
fn build_node_processor(
192+
&self,
193+
skip_masking: bool,
194+
unmasked_root: Option<Rc<dyn SqlNode>>,
195+
) -> Rc<dyn SqlNode> {
196+
let evaluate_sql_processor = MaskedSqlNode::new(
197+
EvaluateSqlNode::new(),
198+
self.group_by_members.clone(),
199+
skip_masking,
200+
unmasked_root.clone(),
201+
);
176202
let auto_prefix_processor = AutoPrefixSqlNode::new(
177203
evaluate_sql_processor.clone(),
178204
self.cube_name_references.clone(),
@@ -188,9 +214,19 @@ impl SqlNodesFactory {
188214
// Wrap the entire measure chain with MaskedSqlNode so masked measures
189215
// are intercepted before aggregation/ungrouped wrapping.
190216
let measure_processor = if self.ungrouped || self.ungrouped_measure {
191-
MaskedSqlNode::new_ungrouped(measure_processor, self.group_by_members.clone())
217+
MaskedSqlNode::new_ungrouped(
218+
measure_processor,
219+
self.group_by_members.clone(),
220+
skip_masking,
221+
unmasked_root.clone(),
222+
)
192223
} else {
193-
MaskedSqlNode::new(measure_processor, self.group_by_members.clone())
224+
MaskedSqlNode::new(
225+
measure_processor,
226+
self.group_by_members.clone(),
227+
skip_masking,
228+
unmasked_root.clone(),
229+
)
194230
};
195231
let measure_processor = self
196232
.add_multi_stage_window_if_needed(measure_processor, measure_filter_processor.clone());

‎rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/sql_nodes/masked.rs‎

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,47 @@ pub struct MaskedSqlNode {
2020
// Full names of the members present in the query GROUP BY. Used to decide
2121
// whether conditional masking can be applied to an aggregate measure.
2222
group_by_members: HashSet<String>,
23+
// When true this node never applies masking and just delegates to `input`.
24+
// Used to build an "unmasked" copy of the whole processor tree that still
25+
// dispatches by member kind (see `unmasked_root`).
26+
skip_masking: bool,
27+
// A kind-dispatching processor (a full `RootSqlNode`-based tree built in
28+
// `skip` mode) used to render mask-filter member references. It routes a
29+
// dimension reference through the dimension chain and a measure reference
30+
// through the measure chain, while skipping masking — so a measure mask
31+
// filter that references a dimension is rendered correctly and a filter
32+
// member that is itself masked does not recurse.
33+
unmasked_root: Option<Rc<dyn SqlNode>>,
2334
}
2435

2536
impl MaskedSqlNode {
26-
pub fn new(input: Rc<dyn SqlNode>, group_by_members: HashSet<String>) -> Rc<Self> {
37+
pub fn new(
38+
input: Rc<dyn SqlNode>,
39+
group_by_members: HashSet<String>,
40+
skip_masking: bool,
41+
unmasked_root: Option<Rc<dyn SqlNode>>,
42+
) -> Rc<Self> {
2743
Rc::new(Self {
2844
input,
2945
ungrouped: false,
3046
group_by_members,
47+
skip_masking,
48+
unmasked_root,
3149
})
3250
}
3351

34-
pub fn new_ungrouped(input: Rc<dyn SqlNode>, group_by_members: HashSet<String>) -> Rc<Self> {
52+
pub fn new_ungrouped(
53+
input: Rc<dyn SqlNode>,
54+
group_by_members: HashSet<String>,
55+
skip_masking: bool,
56+
unmasked_root: Option<Rc<dyn SqlNode>>,
57+
) -> Rc<Self> {
3558
Rc::new(Self {
3659
input,
3760
ungrouped: true,
3861
group_by_members,
62+
skip_masking,
63+
unmasked_root,
3964
})
4065
}
4166

@@ -106,12 +131,20 @@ impl MaskedSqlNode {
106131
)?;
107132
// TODO: support FILTER_PARAMS in mask filter SQL by passing
108133
// proper FiltersContext with filter_params_columns.
109-
// Use self.input as node_processor so member references inside the filter
110-
// resolve through the unmasked chain — prevents recursion through MaskedSqlNode
111-
// when the filter member is itself masked.
134+
// Render the filter through the unmasked, kind-dispatching root so a
135+
// filter member that is a dimension (e.g. an aggregate measure masked by
136+
// a row filter on a group-by dimension) is routed through the dimension
137+
// chain instead of the measure chain (which only accepts measures and
138+
// would otherwise error). `skip` masking also prevents recursion when the
139+
// filter member is itself masked. Falls back to `self.input` if no
140+
// unmasked root was wired in.
141+
let filter_processor = self
142+
.unmasked_root
143+
.clone()
144+
.unwrap_or_else(|| self.input.clone());
112145
let filter_sql = filter_item.to_sql(
113146
visitor,
114-
self.input.clone(),
147+
filter_processor,
115148
query_tools,
116149
templates,
117150
&FiltersContext::default(),
@@ -137,14 +170,16 @@ impl SqlNode for MaskedSqlNode {
137170
node_processor: Rc<dyn SqlNode>,
138171
templates: &PlanSqlTemplates,
139172
) -> Result<String, CubeError> {
140-
if let Some(masked) = self.resolve_mask(
141-
node,
142-
visitor,
143-
node_processor.clone(),
144-
query_tools.clone(),
145-
templates,
146-
)? {
147-
return Ok(masked);
173+
if !self.skip_masking {
174+
if let Some(masked) = self.resolve_mask(
175+
node,
176+
visitor,
177+
node_processor.clone(),
178+
query_tools.clone(),
179+
templates,
180+
)? {
181+
return Ok(masked);
182+
}
148183
}
149184
self.input
150185
.to_sql(visitor, node, query_tools, node_processor, templates)

‎rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,10 @@ impl VisitorContext {
2929
filter_params_columns: HashMap::new(),
3030
reading_pre_aggregation: nodes_factory.reading_pre_aggregation(),
3131
};
32+
let node_processor = nodes_factory.default_node_processor(&query_tools);
3233
Self {
3334
query_tools,
34-
node_processor: nodes_factory.default_node_processor(),
35+
node_processor,
3536
cube_ref_evaluator: Rc::new(nodes_factory.cube_ref_evaluator()),
3637
all_filters,
3738
filters_context,
@@ -48,9 +49,10 @@ impl VisitorContext {
4849
filter_params_columns,
4950
reading_pre_aggregation: nodes_factory.reading_pre_aggregation(),
5051
};
52+
let node_processor = nodes_factory.default_node_processor(&query_tools);
5153
Self {
5254
query_tools,
53-
node_processor: nodes_factory.default_node_processor(),
55+
node_processor,
5456
cube_ref_evaluator: Rc::new(nodes_factory.cube_ref_evaluator()),
5557
all_filters: None,
5658
filters_context,

‎rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ impl QueryTools {
9494
self.masked_members.contains(member_path)
9595
}
9696

97+
/// Whether the query has any masked members at all. `masked_members` is
98+
/// fixed at `try_new`, so this is a cheap, stable signal used to skip
99+
/// building the unmasked reference tree for queries with no masking.
100+
pub fn has_masked_members(&self) -> bool {
101+
!self.masked_members.is_empty()
102+
}
103+
97104
pub fn member_mask_filter(&self, member_path: &str) -> Option<FilterItem> {
98105
self.member_mask_filters.borrow().get(member_path).cloned()
99106
}

‎rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/test_utils/test_context.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ impl TestContext {
319319
let base_tools = self.query_tools.base_tools();
320320
let driver_tools = base_tools.driver_tools(false)?;
321321
let templates = PlanSqlTemplates::try_new(driver_tools, false)?;
322-
let node_processor = nodes_factory.default_node_processor();
322+
let node_processor = nodes_factory.default_node_processor(self.query_tools.query_tools());
323323

324324
visitor.apply(symbol, node_processor, &templates)
325325
}
@@ -345,7 +345,7 @@ impl TestContext {
345345
let base_tools = self.query_tools.base_tools();
346346
let driver_tools = base_tools.driver_tools(false)?;
347347
let templates = PlanSqlTemplates::try_new(driver_tools, false)?;
348-
let node_processor = nodes_factory.default_node_processor();
348+
let node_processor = nodes_factory.default_node_processor(self.query_tools.query_tools());
349349

350350
visitor.apply(symbol, node_processor, &templates)
351351
}

0 commit comments

Comments
 (0)