Skip to content

Commit

Permalink
feat: find matchers function (#73)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
yuanbohan authored Jul 21, 2023
1 parent c774cab commit b8dd489
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 36 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
28 changes: 26 additions & 2 deletions src/label/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,25 @@ 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<String> {
/// 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<String> {
for m in &self.matchers {
if m.name.eq(name) {
return Some(m.value.clone());
}
}
None
}

/// find matchers whose name equals the specified name
pub fn find_matchers(&self, name: &str) -> Vec<Matcher> {
self.matchers
.iter()
.filter(|m| m.name.eq(name))
.cloned()
.collect()
}
}

impl fmt::Display for Matchers {
Expand Down Expand Up @@ -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());
}
}
6 changes: 1 addition & 5 deletions src/label/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
40 changes: 30 additions & 10 deletions src/parser/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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()
)
}
Expand Down Expand Up @@ -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)
)
Expand All @@ -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)
)
}
}
Expand Down Expand Up @@ -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)
)
}
}
Expand Down Expand Up @@ -1405,7 +1406,7 @@ fn check_ast_for_subquery(ex: SubqueryExpr) -> Result<Expr, String> {

fn check_ast_for_vector_selector(ex: VectorSelector) -> Result<Expr, String> {
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
Expand Down Expand Up @@ -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());
}
}
}
49 changes: 36 additions & 13 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -74,26 +73,50 @@ 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
fn needs_split(&self, max: usize) -> bool {
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));
}
}
6 changes: 1 addition & 5 deletions src/parser/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,7 @@ mod tests {

fn assert_cases(cases: Vec<Case>) {
for Case { input, expected } in cases {
assert_eq!(
crate::parser::parse(&input),
expected,
"\n<parse> <{input}> does not match"
);
assert_eq!(expected, crate::parser::parse(&input));
}
}

Expand Down

0 comments on commit b8dd489

Please sign in to comment.