Skip to content

Commit e619372

Browse files
committed
fix(cubesql): Ignore __user IS NOT NULL filter
1 parent 95088cc commit e619372

File tree

2 files changed

+192
-19
lines changed

2 files changed

+192
-19
lines changed

rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs

Lines changed: 121 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,14 @@ impl RewriteRules for FilterRules {
349349
"?filter_aliases",
350350
),
351351
change_user_member("?user"),
352-
self.transform_change_user_eq("?column", "?literal", "?user"),
352+
self.transform_change_user_eq(
353+
"?column",
354+
"?literal",
355+
"?alias_to_cube",
356+
"?members",
357+
"?filter_aliases",
358+
"?user",
359+
),
353360
),
354361
transforming_rewrite(
355362
"change-user-equal-filter",
@@ -360,7 +367,35 @@ impl RewriteRules for FilterRules {
360367
"?filter_aliases",
361368
),
362369
change_user_member("?user"),
363-
self.transform_change_user_eq("?column", "?literal", "?user"),
370+
self.transform_change_user_eq(
371+
"?column",
372+
"?literal",
373+
"?alias_to_cube",
374+
"?members",
375+
"?filter_aliases",
376+
"?user",
377+
),
378+
),
379+
transforming_rewrite(
380+
"user-is-not-null-filter",
381+
filter_replacer(
382+
is_not_null_expr(column_expr("?column")),
383+
"?alias_to_cube",
384+
"?members",
385+
"?filter_aliases",
386+
),
387+
filter_replacer(
388+
literal_bool(true),
389+
"?alias_to_cube",
390+
"?members",
391+
"?filter_aliases",
392+
),
393+
self.transform_user_is_not_null(
394+
"?column",
395+
"?alias_to_cube",
396+
"?members",
397+
"?filter_aliases",
398+
),
364399
),
365400
transforming_rewrite(
366401
"join-field-filter-eq",
@@ -3308,36 +3343,103 @@ impl FilterRules {
33083343
&self,
33093344
column_var: &'static str,
33103345
literal_var: &'static str,
3346+
alias_to_cube_var: &'static str,
3347+
members_var: &'static str,
3348+
filter_aliases_var: &'static str,
33113349
change_user_member_var: &'static str,
33123350
) -> impl Fn(&mut EGraph<LogicalPlanLanguage, LogicalPlanAnalysis>, &mut Subst) -> bool {
3313-
let column_var = column_var.parse().unwrap();
3314-
let literal_var = literal_var.parse().unwrap();
3315-
let change_user_member_var = change_user_member_var.parse().unwrap();
3316-
3351+
let column_var = var!(column_var);
3352+
let literal_var = var!(literal_var);
3353+
let alias_to_cube_var = var!(alias_to_cube_var);
3354+
let members_var = var!(members_var);
3355+
let filter_aliases_var = var!(filter_aliases_var);
3356+
let change_user_member_var = var!(change_user_member_var);
3357+
let meta_context = self.meta_context.clone();
33173358
move |egraph, subst| {
3318-
for literal in var_iter!(egraph[subst[literal_var]], LiteralExprValue) {
3319-
if let ScalarValue::Utf8(Some(change_user)) = literal {
3320-
let specified_user = change_user.clone();
3359+
let literals = var_iter!(egraph[subst[literal_var]], LiteralExprValue)
3360+
.cloned()
3361+
.collect::<Vec<_>>();
3362+
for literal in literals {
3363+
let ScalarValue::Utf8(Some(user_name)) = literal else {
3364+
continue;
3365+
};
33213366

3322-
for column in var_iter!(egraph[subst[column_var]], ColumnExprColumn).cloned() {
3323-
if column.name.eq_ignore_ascii_case("__user") {
3324-
subst.insert(
3325-
change_user_member_var,
3326-
egraph.add(LogicalPlanLanguage::ChangeUserMemberValue(
3327-
ChangeUserMemberValue(specified_user),
3328-
)),
3329-
);
3367+
let aliases_es =
3368+
var_iter!(egraph[subst[filter_aliases_var]], FilterReplacerAliases)
3369+
.cloned()
3370+
.collect::<Vec<_>>();
3371+
for aliases in aliases_es {
3372+
let Some((member_name, cube)) = Self::filter_member_name(
3373+
egraph,
3374+
subst,
3375+
&meta_context,
3376+
alias_to_cube_var,
3377+
column_var,
3378+
members_var,
3379+
&aliases,
3380+
) else {
3381+
continue;
3382+
};
33303383

3331-
return true;
3332-
}
3384+
let user_member_name = format!("{}.__user", cube.name);
3385+
if !member_name.eq_ignore_ascii_case(&user_member_name) {
3386+
continue;
33333387
}
3388+
3389+
subst.insert(
3390+
change_user_member_var,
3391+
egraph.add(LogicalPlanLanguage::ChangeUserMemberValue(
3392+
ChangeUserMemberValue(user_name.clone()),
3393+
)),
3394+
);
3395+
return true;
33343396
}
33353397
}
33363398

33373399
false
33383400
}
33393401
}
33403402

3403+
fn transform_user_is_not_null(
3404+
&self,
3405+
column_var: &'static str,
3406+
alias_to_cube_var: &'static str,
3407+
members_var: &'static str,
3408+
filter_aliases_var: &'static str,
3409+
) -> impl Fn(&mut EGraph<LogicalPlanLanguage, LogicalPlanAnalysis>, &mut Subst) -> bool {
3410+
let column_var = var!(column_var);
3411+
let alias_to_cube_var = var!(alias_to_cube_var);
3412+
let members_var = var!(members_var);
3413+
let filter_aliases_var = var!(filter_aliases_var);
3414+
let meta_context = self.meta_context.clone();
3415+
move |egraph, subst| {
3416+
let aliases_es = var_iter!(egraph[subst[filter_aliases_var]], FilterReplacerAliases)
3417+
.cloned()
3418+
.collect::<Vec<_>>();
3419+
for aliases in aliases_es {
3420+
let Some((member_name, cube)) = Self::filter_member_name(
3421+
egraph,
3422+
subst,
3423+
&meta_context,
3424+
alias_to_cube_var,
3425+
column_var,
3426+
members_var,
3427+
&aliases,
3428+
) else {
3429+
continue;
3430+
};
3431+
3432+
let user_member_name = format!("{}.__user", cube.name);
3433+
if !member_name.eq_ignore_ascii_case(&user_member_name) {
3434+
continue;
3435+
}
3436+
3437+
return true;
3438+
}
3439+
false
3440+
}
3441+
}
3442+
33413443
// Transform ?expr IN (?literal) to ?expr = ?literal
33423444
fn transform_filter_in_to_equal(
33433445
&self,

rust/cubesql/cubesql/src/compile/test/test_user_change.rs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,36 @@ async fn test_change_user_via_in_filter_thoughtspot() {
110110
assert_eq!(cube_scan.request, expected_request);
111111
}
112112

113+
#[tokio::test]
114+
async fn test_change_user_via_filter_powerbi() {
115+
init_testing_logger();
116+
117+
let query_plan = convert_select_to_query_plan(
118+
"SELECT COUNT(*) as cnt FROM KibanaSampleDataEcommerce WHERE NOT __user IS NULL AND __user = 'gopher'".to_string(),
119+
DatabaseProtocol::PostgreSQL,
120+
)
121+
.await;
122+
123+
let cube_scan = query_plan.as_logical_plan().find_cube_scan();
124+
125+
assert_eq!(cube_scan.options.change_user, Some("gopher".to_string()));
126+
127+
assert_eq!(
128+
cube_scan.request,
129+
V1LoadRequestQuery {
130+
measures: Some(vec!["KibanaSampleDataEcommerce.count".to_string(),]),
131+
segments: Some(vec![]),
132+
dimensions: Some(vec![]),
133+
time_dimensions: None,
134+
order: Some(vec![]),
135+
limit: None,
136+
offset: None,
137+
filters: None,
138+
ungrouped: None,
139+
}
140+
)
141+
}
142+
113143
#[tokio::test]
114144
async fn test_change_user_via_filter_and() {
115145
let query_plan = convert_select_to_query_plan(
@@ -192,6 +222,47 @@ async fn test_user_with_join() {
192222
assert_eq!(cube_scan.options.change_user, Some("foo".to_string()))
193223
}
194224

225+
#[tokio::test]
226+
async fn test_change_user_via_filter_with_alias() {
227+
init_testing_logger();
228+
229+
let query_plan = convert_select_to_query_plan(
230+
r#"
231+
SELECT "k"."cnt" AS "cnt"
232+
FROM (
233+
SELECT
234+
COUNT(*) AS "cnt",
235+
"__user" AS "user"
236+
FROM "KibanaSampleDataEcommerce"
237+
GROUP BY 2
238+
) AS "k"
239+
WHERE "k"."user" = 'gopher'
240+
"#
241+
.to_string(),
242+
DatabaseProtocol::PostgreSQL,
243+
)
244+
.await;
245+
246+
let cube_scan = query_plan.as_logical_plan().find_cube_scan();
247+
248+
assert_eq!(cube_scan.options.change_user, Some("gopher".to_string()));
249+
250+
assert_eq!(
251+
cube_scan.request,
252+
V1LoadRequestQuery {
253+
measures: Some(vec!["KibanaSampleDataEcommerce.count".to_string(),]),
254+
segments: Some(vec![]),
255+
dimensions: Some(vec![]),
256+
time_dimensions: None,
257+
order: Some(vec![]),
258+
limit: None,
259+
offset: None,
260+
filters: None,
261+
ungrouped: None,
262+
}
263+
)
264+
}
265+
195266
/// This should test that query with CubeScanWrapper uses proper change_user for both SQL generation and execution calls
196267
#[tokio::test]
197268
async fn test_user_change_sql_generation() {

0 commit comments

Comments
 (0)