fix review issues

GreptimeTeam · May 15, 2024 · ada6424 · ada6424
1 parent 76bc3d1
commit ada6424
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 45 deletions.
diff --git a/src/parser/lex.rs b/src/parser/lex.rs
@@ -564,12 +564,17 @@ impl Lexer {
         match self.pop() {
             Some('#') => State::LineComment,
             Some(',') => State::Lexeme(T_COMMA),
-            Some('o') => match self.peek() {
-                Some('r') => {
+            Some('o') =>  {
+                if let Some('r') =  self.peek() {
                     self.pop();
-                    State::Lexeme(T_LOR)
+                    if let Some(' ') = self.peek() {
+                        State::Lexeme(T_LOR)
+                    }  else {
+                        State::Identifier
+                    }
+                } else {
+                    State::Identifier
                 }
-                _ => State::Identifier,
             },
             Some(ch) if ch.is_ascii_whitespace() => State::Space,
             Some(ch) if is_alpha(ch) => State::Identifier,

diff --git a/src/parser/parse.rs b/src/parser/parse.rs
@@ -37,7 +37,6 @@ mod tests {
     use regex::Regex;
 
     use crate::label::{Labels, MatchOp, Matcher, Matchers, METRIC_NAME};
-    use crate::parser;
     use crate::parser::function::get_function;
     use crate::parser::{
         token, AtModifier as At, BinModifier, Expr, FunctionArgs, LabelModifier, Offset,
@@ -2118,7 +2117,7 @@ mod tests {
     }
 
     #[test]
-    fn test_label_or() {
+    fn test_or_filters() {
         let cases = vec![
             (r#"foo{label1="1" or label2="2"}"#, {
                 let matchers = Matchers::new(vec![
@@ -2127,66 +2126,41 @@ mod tests {
                 ]);
                 Expr::new_vector_selector(Some(String::from("foo")), matchers)
             }),
-            (r#"foo{label1="1", label2="2"}"#, {
+            (r#"foo{label1="1" or or="or"}"#, {
                 let matchers = Matchers::new(vec![
                     Matcher::new(MatchOp::Equal, "label1", "1"),
-                    Matcher::new(MatchOp::Equal, "label2", "2"),
+                    Matcher::new_or(MatchOp::Equal, "or", "or"),
                 ]);
                 Expr::new_vector_selector(Some(String::from("foo")), matchers)
             }),
-            (r#"foo{label1="1" or label2="2", label3="3"}"#, {
+            (r#"foo{label1="1" or label2="2" or label3="3"}"#, {
                 let matchers = Matchers::new(vec![
                     Matcher::new(MatchOp::Equal, "label1", "1"),
                     Matcher::new_or(MatchOp::Equal, "label2", "2"),
-                    Matcher::new(MatchOp::Equal, "label3", "3"),
+                    Matcher::new_or(MatchOp::Equal, "label3", "3"),
                 ]);
                 Expr::new_vector_selector(Some(String::from("foo")), matchers)
             }),
-            (r#"foo{label1="1", label2="2" or label3="3"}"#, {
+            (r#"foo{label1="1" or label2="2" or label3="3" or label4="4"}"#, {
                 let matchers = Matchers::new(vec![
                     Matcher::new(MatchOp::Equal, "label1", "1"),
-                    Matcher::new(MatchOp::Equal, "label2", "2"),
+                    Matcher::new_or(MatchOp::Equal, "label2", "2"),
                     Matcher::new_or(MatchOp::Equal, "label3", "3"),
+                    Matcher::new_or(MatchOp::Equal, "label4", "4"),
                 ]);
                 Expr::new_vector_selector(Some(String::from("foo")), matchers)
             }),
-            (
-                r#"foo{label1="1", label2="2" or label3="3", label4="4"}"#,
-                {
-                    let matchers = Matchers::new(vec![
-                        Matcher::new(MatchOp::Equal, "label1", "1"),
-                        Matcher::new(MatchOp::Equal, "label2", "2"),
-                        Matcher::new_or(MatchOp::Equal, "label3", "3"),
-                        Matcher::new(MatchOp::Equal, "label4", "4"),
-                    ]);
-                    Expr::new_vector_selector(Some(String::from("foo")), matchers)
-                },
-            ),
         ];
         assert_cases(Case::new_result_cases(cases));
 
-        let promql = r#"a{on="1" or label2="2"}"#;
-        let expr = parser::parse(promql).unwrap();
-        assert_eq!(expr.to_string(), promql);
-
-        let promql = r#"a{label1="1",label2="2"}"#;
-        let expr = parser::parse(promql).unwrap();
-        assert_eq!(expr.to_string(), promql);
-
-        let promql = r#"a{label1="1" or label2="2",label3="3"}"#;
-        let expr = parser::parse(promql).unwrap();
-        assert_eq!(expr.to_string(), promql);
-
-        let promql = r#"a{label1="1",label2="2" or label3="3"}"#;
-        let expr = parser::parse(promql).unwrap();
-        assert_eq!(expr.to_string(), promql);
 
-        let promql = r#"a{label1="1",label2="2" or label3="3",label4="4"}"#;
-        let expr = parser::parse(promql).unwrap();
-        assert_eq!(expr.to_string(), promql);
+        let fail_cases = vec![
+            (r#"foo{or}"#, r#"invalid label matcher, expected label matching operator after 'or'"#),
+            (r#"foo{label1="1" or}"#, INVALID_QUERY_INFO),
+            (r#"foo{or label1="1"}"#, INVALID_QUERY_INFO),
+            (r#"foo{label1="1" or or label2="2"}"#, INVALID_QUERY_INFO),
+        ];
+        assert_cases(Case::new_fail_cases(fail_cases));
 
-        let promql = r#"a{o="1",o1="2" or o2="3",o3="4"}"#;
-        let expr = parser::parse(promql).unwrap();
-        assert_eq!(expr.to_string(), promql);
     }
 }