From b8dd489e5f5968443397f446788380c9384624c0 Mon Sep 17 00:00:00 2001 From: yuanbohan Date: Fri, 21 Jul 2023 22:06:22 +0800 Subject: [PATCH] feat: find matchers function (#73) * feat: find_matchers function * chore: increase code coverage * chore: update minor version to 0.2.0 for one breaking function * chore: increase the coverage of Prettier::format --- Cargo.toml | 2 +- src/label/matcher.rs | 28 +++++++++++++++++++++++-- src/label/mod.rs | 6 +----- src/parser/ast.rs | 40 +++++++++++++++++++++++++++--------- src/parser/mod.rs | 49 ++++++++++++++++++++++++++++++++------------ src/parser/parse.rs | 6 +----- 6 files changed, 95 insertions(+), 36 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f19f9a0..11b22d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ name = "promql-parser" readme = "README.md" description = "Parse PromQL query into AST" repository = "https://github.com/GreptimeTeam/promql-parser" -version = "0.1.4" +version = "0.2.0" edition = "2021" authors = ["The GreptimeDB Project Developers"] keywords = ["prometheus", "promql", "parser"] diff --git a/src/label/matcher.rs b/src/label/matcher.rs index 2e05e02..a915c99 100644 --- a/src/label/matcher.rs +++ b/src/label/matcher.rs @@ -149,8 +149,9 @@ impl Matchers { self.matchers.is_empty() || self.matchers.iter().all(|m| m.is_match("")) } - /// find the matcher's value whose name equals the specified name. - pub fn find_matcher(&self, name: &str) -> Option { + /// find the matcher's value whose name equals the specified name. This function + /// is designed to prepare error message of invalid promql expression. + pub fn find_matcher_value(&self, name: &str) -> Option { for m in &self.matchers { if m.name.eq(name) { return Some(m.value.clone()); @@ -158,6 +159,15 @@ impl Matchers { } None } + + /// find matchers whose name equals the specified name + pub fn find_matchers(&self, name: &str) -> Vec { + self.matchers + .iter() + .filter(|m| m.name.eq(name)) + .cloned() + .collect() + } } impl fmt::Display for Matchers { @@ -441,4 +451,18 @@ mod tests { )) ); } + + #[test] + fn test_find_matchers() { + let matchers = Matchers::empty() + .append(Matcher::new(MatchOp::Equal, "foo", "bar")) + .append(Matcher::new(MatchOp::NotEqual, "foo", "bar")) + .append(Matcher::new_matcher(T_EQL_REGEX, "foo".into(), "bar".into()).unwrap()) + .append(Matcher::new_matcher(T_NEQ_REGEX, "foo".into(), "bar".into()).unwrap()) + .append(Matcher::new(MatchOp::Equal, "FOO", "bar")) + .append(Matcher::new(MatchOp::NotEqual, "bar", "bar")); + + let ms = matchers.find_matchers("foo"); + assert_eq!(4, ms.len()); + } } diff --git a/src/label/mod.rs b/src/label/mod.rs index 264d1be..1bba6d2 100644 --- a/src/label/mod.rs +++ b/src/label/mod.rs @@ -121,11 +121,7 @@ mod tests { let lb2 = Labels::new(lb2); let intersection: HashSet<_> = lb1.intersect(&lb2).labels.into_iter().collect(); let expect: HashSet<_> = common.iter().map(|s| s.to_string()).collect(); - assert_eq!( - expect, intersection, - "{:?} intersect {:?} does not eq {:?}", - lb1, lb2, common - ) + assert_eq!(expect, intersection) } } } diff --git a/src/parser/ast.rs b/src/parser/ast.rs index e744051..45f5787 100644 --- a/src/parser/ast.rs +++ b/src/parser/ast.rs @@ -17,7 +17,8 @@ use crate::parser::token::{ self, token_display, T_BOTTOMK, T_COUNT_VALUES, T_END, T_QUANTILE, T_START, T_TOPK, }; use crate::parser::{ - Function, FunctionArgs, Prettier, Token, TokenId, TokenType, ValueType, MAX_CHARACTERS_PER_LINE, + indent, Function, FunctionArgs, Prettier, Token, TokenId, TokenType, ValueType, + MAX_CHARACTERS_PER_LINE, }; use crate::util::display_duration; use std::fmt::{self, Write}; @@ -365,12 +366,12 @@ impl fmt::Display for AggregateExpr { impl Prettier for AggregateExpr { fn format(&self, level: usize, max: usize) -> String { - let mut s = format!("{}{}(\n", self.indent(level), self.get_op_string()); + let mut s = format!("{}{}(\n", indent(level), self.get_op_string()); if let Some(param) = &self.param { writeln!(s, "{},", param.pretty(level + 1, max)).unwrap(); } writeln!(s, "{}", self.expr.pretty(level + 1, max)).unwrap(); - write!(s, "{})", self.indent(level)).unwrap(); + write!(s, "{})", indent(level)).unwrap(); s } } @@ -391,7 +392,7 @@ impl Prettier for UnaryExpr { fn pretty(&self, level: usize, max: usize) -> String { format!( "{}-{}", - self.indent(level), + indent(level), self.expr.pretty(level, max).trim_start() ) } @@ -466,7 +467,7 @@ impl Prettier for BinaryExpr { format!( "{}\n{}{}\n{}", self.lhs.pretty(level + 1, max), - self.indent(level), + indent(level), self.get_op_matching_string(), self.rhs.pretty(level + 1, max) ) @@ -488,9 +489,9 @@ impl Prettier for ParenExpr { fn format(&self, level: usize, max: usize) -> String { format!( "{}(\n{}\n{})", - self.indent(level), + indent(level), self.expr.pretty(level + 1, max), - self.indent(level) + indent(level) ) } } @@ -802,10 +803,10 @@ impl Prettier for Call { fn format(&self, level: usize, max: usize) -> String { format!( "{}{}(\n{}\n{})", - self.indent(level), + indent(level), self.func.name, self.args.pretty(level + 1, max), - self.indent(level) + indent(level) ) } } @@ -1405,7 +1406,7 @@ fn check_ast_for_subquery(ex: SubqueryExpr) -> Result { fn check_ast_for_vector_selector(ex: VectorSelector) -> Result { match ex.name { - Some(ref name) => match ex.matchers.find_matcher(METRIC_NAME) { + Some(ref name) => match ex.matchers.find_matcher_value(METRIC_NAME) { Some(val) => Err(format!( "metric name must not be set twice: '{}' or '{}'", name, val @@ -2418,4 +2419,23 @@ or assert_eq!(expect, expr.unwrap().pretty(0, 10)); } } + + #[test] + fn test_call_expr_prettify() { + let cases = vec![ + ("vector_selector", "vector_selector"), + ( + r#"vector_selector{fooooooooooooooooo="barrrrrrrrrrrrrrrrrrr",barrrrrrrrrrrrrrrrrrr="fooooooooooooooooo",process_name="alertmanager"}"#, + r#"vector_selector{barrrrrrrrrrrrrrrrrrr="fooooooooooooooooo",fooooooooooooooooo="barrrrrrrrrrrrrrrrrrr",process_name="alertmanager"}"#, + ), + ( + r#"matrix_selector{fooooooooooooooooo="barrrrrrrrrrrrrrrrrrr",barrrrrrrrrrrrrrrrrrr="fooooooooooooooooo",process_name="alertmanager"}[1y2w3d]"#, + r#"matrix_selector{barrrrrrrrrrrrrrrrrrr="fooooooooooooooooo",fooooooooooooooooo="barrrrrrrrrrrrrrrrrrr",process_name="alertmanager"}[382d]"#, + ), + ]; + + for (input, expect) in cases { + assert_eq!(expect, crate::parser::parse(&input).unwrap().prettify()); + } + } } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index b9de08a..c772d40 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -41,8 +41,7 @@ pub use token::{Token, TokenId, TokenType}; pub use value::{Value, ValueType}; // FIXME: show more helpful error message to some invalid promql queries. -pub const INVALID_QUERY_INFO: &str = "invalid promql query"; - +const INVALID_QUERY_INFO: &str = "invalid promql query"; const INDENT_STR: &str = " "; const MAX_CHARACTERS_PER_LINE: usize = 100; @@ -74,22 +73,13 @@ pub trait Prettier: std::fmt::Display { if self.needs_split(max) { self.format(level, max) } else { - self.default_format(level) + format!("{}{self}", indent(level)) } } - fn indent(&self, n: usize) -> String { - INDENT_STR.repeat(n) - } - - /// default_format is designed not for override - fn default_format(&self, level: usize) -> String { - format!("{}{self}", self.indent(level)) - } - /// override format if expr needs to be splited into multiple lines fn format(&self, level: usize, _max: usize) -> String { - self.default_format(level) + format!("{}{self}", indent(level)) } /// override needs_split to return false, in order not to split multiple lines @@ -97,3 +87,36 @@ pub trait Prettier: std::fmt::Display { self.to_string().len() > max } } + +fn indent(n: usize) -> String { + INDENT_STR.repeat(n) +} + +#[cfg(test)] +mod tests { + use super::*; + + struct Pretty(String); + + impl std::fmt::Display for Pretty { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } + } + + impl Prettier for Pretty {} + + #[test] + fn test_prettier_trait() { + let max = 10; + let level = 1; + + let p = Pretty("demo".into()); + assert!(!p.needs_split(max)); + assert_eq!(p.format(level, max), p.pretty(level, max)); + + let p = Pretty("demo_again.".into()); + assert!(p.needs_split(max)); + assert_eq!(p.format(level, max), p.pretty(level, max)); + } +} diff --git a/src/parser/parse.rs b/src/parser/parse.rs index 4f1ec86..796095d 100644 --- a/src/parser/parse.rs +++ b/src/parser/parse.rs @@ -82,11 +82,7 @@ mod tests { fn assert_cases(cases: Vec) { for Case { input, expected } in cases { - assert_eq!( - crate::parser::parse(&input), - expected, - "\n <{input}> does not match" - ); + assert_eq!(expected, crate::parser::parse(&input)); } }