Skip to content

Commit aee6b42

Browse files
nicosuavenicosuave
committed
Handle qualified warning scan edge cases
1 parent ed4912e commit aee6b42

1 file changed

Lines changed: 119 additions & 56 deletions

File tree

‎yardstick-rs/src/sql/measures.rs‎

Lines changed: 119 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -5933,6 +5933,40 @@ fn read_quoted_identifier(where_clause: &str, mut i: usize) -> (String, usize) {
59335933
(token, i)
59345934
}
59355935

5936+
fn read_warning_dotted_identifier(
5937+
where_clause: &str,
5938+
first_token: String,
5939+
mut i: usize,
5940+
) -> Option<(String, String, usize)> {
5941+
let bytes = where_clause.as_bytes();
5942+
let mut parts = vec![first_token];
5943+
loop {
5944+
let dot_pos = skip_sql_whitespace(bytes, i);
5945+
if dot_pos >= bytes.len() || bytes[dot_pos] != b'.' {
5946+
break;
5947+
}
5948+
let next_start = skip_sql_whitespace(bytes, dot_pos + 1);
5949+
if next_start >= bytes.len() {
5950+
break;
5951+
}
5952+
let (part, next_end) = if bytes[next_start] == b'"' {
5953+
read_quoted_identifier(where_clause, next_start)
5954+
} else if (bytes[next_start] as char).is_alphabetic() || bytes[next_start] == b'_' {
5955+
read_unquoted_identifier(where_clause, next_start)
5956+
} else {
5957+
break;
5958+
};
5959+
parts.push(part);
5960+
i = next_end;
5961+
}
5962+
5963+
if parts.len() < 2 {
5964+
return None;
5965+
}
5966+
let name = parts.pop().unwrap();
5967+
Some((parts.join("."), name, i))
5968+
}
5969+
59365970
fn skip_sql_whitespace(bytes: &[u8], mut i: usize) -> usize {
59375971
while i < bytes.len() && bytes[i].is_ascii_whitespace() {
59385972
i += 1;
@@ -6150,34 +6184,20 @@ fn extract_where_filter_identifiers(where_clause: &str) -> Vec<WarningFilterIden
61506184
}
61516185
b'"' => {
61526186
let (token, after_token) = read_quoted_identifier(where_clause, i);
6153-
let after_ws = skip_sql_whitespace(bytes, after_token);
6154-
if after_ws < bytes.len() && bytes[after_ws] == b'.' {
6155-
let next_start = skip_sql_whitespace(bytes, after_ws + 1);
6156-
if next_start < bytes.len() && bytes[next_start] == b'"' {
6157-
let (qualified_token, next_end) =
6158-
read_quoted_identifier(where_clause, next_start);
6187+
if let Some((qualifier, qualified_token, next_end)) =
6188+
read_warning_dotted_identifier(where_clause, token.clone(), after_token)
6189+
{
6190+
let next_after_ws = skip_sql_whitespace(bytes, next_end);
6191+
if !is_warning_identifier_keyword(&qualified_token)
6192+
&& !(next_after_ws < bytes.len() && bytes[next_after_ws] == b'(')
6193+
{
61596194
identifiers.push(WarningFilterIdentifier::new(
6160-
Some(&token),
6195+
Some(&qualifier),
61616196
&qualified_token,
61626197
));
6163-
i = next_end;
6164-
continue;
6165-
}
6166-
if next_start < bytes.len()
6167-
&& (((bytes[next_start] as char).is_alphabetic())
6168-
|| bytes[next_start] == b'_')
6169-
{
6170-
let (qualified_token, next_end) =
6171-
read_unquoted_identifier(where_clause, next_start);
6172-
if !is_warning_identifier_keyword(&qualified_token) {
6173-
identifiers.push(WarningFilterIdentifier::new(
6174-
Some(&token),
6175-
&qualified_token,
6176-
));
6177-
}
6178-
i = next_end;
6179-
continue;
61806198
}
6199+
i = next_end;
6200+
continue;
61816201
}
61826202
identifiers.push(WarningFilterIdentifier::new(None, &token));
61836203
i = after_token;
@@ -6186,41 +6206,21 @@ fn extract_where_filter_identifiers(where_clause: &str) -> Vec<WarningFilterIden
61866206
let (token, after_token) = read_unquoted_identifier(where_clause, i);
61876207
let after_ws = skip_sql_whitespace(bytes, after_token);
61886208

6189-
if after_ws < bytes.len() && bytes[after_ws] == b'.' {
6190-
let next_start = skip_sql_whitespace(bytes, after_ws + 1);
6191-
if next_start < bytes.len() && bytes[next_start] == b'"' {
6192-
let (qualified_token, next_end) =
6193-
read_quoted_identifier(where_clause, next_start);
6209+
if let Some((qualifier, qualified_token, next_end)) =
6210+
read_warning_dotted_identifier(where_clause, token.clone(), after_token)
6211+
{
6212+
let next_after_ws = skip_sql_whitespace(bytes, next_end);
6213+
if !is_warning_identifier_keyword(&qualified_token)
6214+
&& !is_warning_typed_literal_start(&qualified_token, bytes, next_after_ws)
6215+
&& !(next_after_ws < bytes.len() && bytes[next_after_ws] == b'(')
6216+
{
61946217
identifiers.push(WarningFilterIdentifier::new(
6195-
Some(&token),
6218+
Some(&qualifier),
61966219
&qualified_token,
61976220
));
6198-
i = next_end;
6199-
continue;
6200-
}
6201-
if next_start < bytes.len()
6202-
&& (((bytes[next_start] as char).is_alphabetic())
6203-
|| bytes[next_start] == b'_')
6204-
{
6205-
let (qualified_token, next_end) =
6206-
read_unquoted_identifier(where_clause, next_start);
6207-
let next_after_ws = skip_sql_whitespace(bytes, next_end);
6208-
if !is_warning_identifier_keyword(&qualified_token)
6209-
&& !is_warning_typed_literal_start(
6210-
&qualified_token,
6211-
bytes,
6212-
next_after_ws,
6213-
)
6214-
&& !(next_after_ws < bytes.len() && bytes[next_after_ws] == b'(')
6215-
{
6216-
identifiers.push(WarningFilterIdentifier::new(
6217-
Some(&token),
6218-
&qualified_token,
6219-
));
6220-
}
6221-
i = next_end;
6222-
continue;
62236221
}
6222+
i = next_end;
6223+
continue;
62246224
}
62256225

62266226
if is_warning_typed_literal_start(&token, bytes, after_ws) {
@@ -6327,13 +6327,35 @@ fn normalized_warning_expression(sql: &str) -> String {
63276327
.collect()
63286328
}
63296329

6330+
fn normalized_warning_expression_search_text(sql: &str) -> String {
6331+
let bytes = sql.as_bytes();
6332+
let mut result = String::new();
6333+
let mut i = 0;
6334+
while i < bytes.len() {
6335+
if let Some(next) = skip_warning_identifier_comment(bytes, i) {
6336+
i = next;
6337+
continue;
6338+
}
6339+
if bytes[i] == b'\'' {
6340+
i = skip_quoted_sql(bytes, i, b'\'');
6341+
continue;
6342+
}
6343+
let c = bytes[i] as char;
6344+
if !c.is_whitespace() {
6345+
result.extend(c.to_lowercase());
6346+
}
6347+
i += 1;
6348+
}
6349+
result
6350+
}
6351+
63306352
fn warning_expression_in_clause(expr: &str, clause: &str) -> bool {
63316353
let needle = normalized_warning_expression(expr);
63326354
if needle.is_empty() {
63336355
return false;
63346356
}
63356357

6336-
let haystack = normalized_warning_expression(clause);
6358+
let haystack = normalized_warning_expression_search_text(clause);
63376359
let mut search_from = 0;
63386360
while let Some(relative_idx) = haystack[search_from..].find(&needle) {
63396361
let idx = search_from + relative_idx;
@@ -8060,6 +8082,27 @@ FROM orders"#;
80608082
assert!(warning.is_none());
80618083
}
80628084

8085+
#[test]
8086+
fn test_at_all_ignores_non_source_three_part_qualified_where_filter_dimension() {
8087+
let source_dims = HashSet::from([
8088+
normalize_identifier_name("year"),
8089+
normalize_identifier_name("region"),
8090+
]);
8091+
let source_qualifiers = HashSet::from([
8092+
normalize_identifier_name("sales_v"),
8093+
normalize_identifier_name("s"),
8094+
]);
8095+
let warning = warning_for_at_all_ungrouped_where_with_qualifiers(
8096+
"revenue",
8097+
&[ContextModifier::All("region".to_string())],
8098+
Some("main.calendar.year = 2023"),
8099+
&["region".to_string()],
8100+
&source_dims,
8101+
&source_qualifiers,
8102+
);
8103+
assert!(warning.is_none());
8104+
}
8105+
80638106
#[test]
80648107
fn test_at_all_ignores_interval_unit_as_where_filter_dimension() {
80658108
let source_dims = HashSet::from([
@@ -8331,6 +8374,26 @@ FROM orders"#;
83318374
assert!(warning.contains("order_date"));
83328375
}
83338376

8377+
#[test]
8378+
fn test_at_all_expression_set_does_not_match_expression_only_in_comment() {
8379+
let source_dims = HashSet::from([
8380+
normalize_identifier_name("order_date"),
8381+
normalize_identifier_name("region"),
8382+
]);
8383+
let warning = warning_for_at_all_ungrouped_where(
8384+
"revenue",
8385+
&[
8386+
ContextModifier::All("region".to_string()),
8387+
ContextModifier::Set("MONTH(order_date)".to_string(), "2".to_string()),
8388+
],
8389+
Some("order_date = DATE '2023-02-10' /* MONTH(order_date) */"),
8390+
&["region".to_string()],
8391+
&source_dims,
8392+
)
8393+
.unwrap();
8394+
assert!(warning.contains("order_date"));
8395+
}
8396+
83348397
#[test]
83358398
fn test_at_all_preserves_warning_through_fallback_expansion_result() {
83368399
let mut result = AggregateExpandResult {

0 commit comments

Comments
 (0)