From e9f3447125c71a55eca29aa627cb24db8d0949c4 Mon Sep 17 00:00:00 2001
From: yuanbohan <yuanbo.han@gmail.com>
Date: Tue, 18 Jul 2023 19:15:06 +0800
Subject: [PATCH] feat: Labels to Vec<String> instead of HashSet<String>

---
 src/label/matcher.rs |   1 +
 src/label/mod.rs     |  63 +++++-
 src/parser/ast.rs    | 144 ++++++++------
 src/parser/parse.rs  | 462 +++++--------------------------------------
 src/parser/promql.y  |  11 +-
 5 files changed, 196 insertions(+), 485 deletions(-)

diff --git a/src/label/matcher.rs b/src/label/matcher.rs
index 5ced7db..d5be943 100644
--- a/src/label/matcher.rs
+++ b/src/label/matcher.rs
@@ -16,6 +16,7 @@ use std::collections::HashSet;
 use std::fmt;
 use std::hash::{Hash, Hasher};
 
+use crate::label::METRIC_NAME;
 use crate::parser::token::{TokenId, T_EQL, T_EQL_REGEX, T_NEQ, T_NEQ_REGEX};
 use regex::Regex;
 
diff --git a/src/label/mod.rs b/src/label/mod.rs
index 6a0af3a..2d2ca92 100644
--- a/src/label/mod.rs
+++ b/src/label/mod.rs
@@ -15,7 +15,6 @@
 //! Label matchers and Well-known label names used by Prometheus components.
 
 mod matcher;
-
 pub use matcher::{MatchOp, Matcher, Matchers};
 use std::collections::HashSet;
 
@@ -30,4 +29,64 @@ pub const INSTANCE_NAME: &str = "instance";
 
 pub type Label = String;
 /// Unordered set for a group of labels.
-pub type Labels = HashSet<Label>;
+pub type Labels = Vec<Label>;
+
+pub fn new_labels(ls: Vec<&str>) -> Labels {
+    ls.iter().map(|s| s.to_string()).collect()
+}
+
+pub fn is_labels_joint(ls1: &Labels, ls2: &Labels) -> bool {
+    let s1: HashSet<&String> = ls1.iter().collect();
+    let s2: HashSet<&String> = ls2.iter().collect();
+
+    !s1.is_disjoint(&s2)
+}
+
+pub fn intersect_labels(ls1: &Labels, ls2: &Labels) -> Labels {
+    let s1: HashSet<&String> = ls1.iter().collect();
+    let s2: HashSet<&String> = ls2.iter().collect();
+
+    s1.intersection(&s2).map(|s| s.to_string()).collect()
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_is_labels_joint() {
+        let cases = vec![
+            (vec!["foo"], vec!["bar"], false),
+            (vec!["foo"], vec!["foo", "bar"], true),
+            (vec!["foo"], vec!["foo"], true),
+        ];
+
+        for (lb1, lb2, is) in cases {
+            let lb1: Labels = new_labels(lb1);
+            let lb2: Labels = new_labels(lb2);
+            assert_eq!(is, is_labels_joint(&lb1, &lb2), "{:?} and {:?}", lb1, lb2)
+        }
+    }
+
+    #[test]
+    fn test_intersect_labels() {
+        let cases = vec![
+            (vec!["foo"], vec!["bar"], vec![]),
+            (vec!["foo"], vec!["foo", "bar"], vec!["foo"]),
+            (vec!["foo"], vec!["foo"], vec!["foo"]),
+            (vec!["foo", "bar"], vec!["bar", "foo"], vec!["foo", "bar"]),
+        ];
+
+        for (lb1, lb2, common) in cases {
+            let lb1: Labels = new_labels(lb1);
+            let lb2: Labels = new_labels(lb2);
+            let intersection: HashSet<_> = intersect_labels(&lb1, &lb2).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
+            )
+        }
+    }
+}
diff --git a/src/parser/ast.rs b/src/parser/ast.rs
index 46e884e..c571118 100644
--- a/src/parser/ast.rs
+++ b/src/parser/ast.rs
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::label::{Labels, Matchers, METRIC_NAME};
+use crate::label::{intersect_labels, is_labels_joint, new_labels, Labels, Matchers, METRIC_NAME};
 use crate::parser::token::{
     self, token_display, T_BOTTOMK, T_COUNT_VALUES, T_END, T_QUANTILE, T_START, T_TOPK,
 };
@@ -56,6 +56,31 @@ impl LabelModifier {
     pub fn is_on(&self) -> bool {
         matches!(*self, LabelModifier::Include(_))
     }
+
+    pub fn include(ls: Vec<&str>) -> Self {
+        Self::Include(new_labels(ls))
+    }
+
+    pub fn exclude(ls: Vec<&str>) -> Self {
+        Self::Exclude(new_labels(ls))
+    }
+}
+
+impl fmt::Display for LabelModifier {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if self.is_on() && self.labels().is_empty() {
+            write!(f, "")
+        } else {
+            let op = if self.is_on() { "by" } else { "without" };
+            let labels = self
+                .labels()
+                .iter()
+                .map(|e| e.to_string())
+                .collect::<Vec<String>>()
+                .join(", ");
+            write!(f, "{op} ({labels})")
+        }
+    }
 }
 
 /// The label list provided with the group_left or group_right modifier contains
@@ -77,6 +102,14 @@ impl VectorMatchCardinality {
             VectorMatchCardinality::OneToOne => None,
         }
     }
+
+    pub fn many_to_one(ls: Vec<&str>) -> Self {
+        Self::ManyToOne(new_labels(ls))
+    }
+
+    pub fn one_to_many(ls: Vec<&str>) -> Self {
+        Self::OneToMany(new_labels(ls))
+    }
 }
 
 /// Binary Expr Modifier
@@ -174,14 +207,16 @@ impl BinModifier {
     }
 
     pub fn is_labels_joint(&self) -> bool {
-        matches!((self.card.labels(), &self.matching),
-                 (Some(labels), Some(matching)) if !matching.labels().is_disjoint(labels))
+        matches!(
+            (self.card.labels(), &self.matching),
+            (Some(labels), Some(matching)) if is_labels_joint(matching.labels(), labels)
+        )
     }
 
-    pub fn intersect_labels(&self) -> Option<Vec<&String>> {
+    pub fn intersect_labels(&self) -> Option<Vec<String>> {
         if let Some(labels) = self.card.labels() {
             if let Some(matching) = &self.matching {
-                return Some(matching.labels().intersection(labels).collect());
+                return Some(intersect_labels(matching.labels(), labels));
             }
         };
         None
@@ -330,6 +365,34 @@ pub struct AggregateExpr {
     pub modifier: Option<LabelModifier>,
 }
 
+impl fmt::Display for AggregateExpr {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        let mut v: Vec<String> = vec![token_display(self.op.id()).into()];
+
+        // modifier
+        {
+            if let Some(modifier) = &self.modifier {
+                let s = modifier.to_string();
+                if !s.is_empty() {
+                    v.push(s);
+                }
+            }
+        }
+
+        // body
+        {
+            let mut body = String::from("(");
+            if let Some(param) = &self.param {
+                body.push_str(&format!("{param}, "));
+            }
+            body.push_str(&format!("{})", self.expr));
+            v.push(body)
+        }
+
+        write!(f, "{}", v.join(" "))
+    }
+}
+
 /// UnaryExpr will negate the expr
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct UnaryExpr {
@@ -374,7 +437,7 @@ impl BinaryExpr {
     }
 
     /// intersect labels of card and matching
-    pub fn intersect_labels(&self) -> Option<Vec<&String>> {
+    pub fn intersect_labels(&self) -> Option<Vec<String>> {
         self.modifier
             .as_ref()
             .and_then(|modifier| modifier.intersect_labels())
@@ -892,40 +955,7 @@ impl Neg for Expr {
 impl fmt::Display for Expr {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            Expr::Aggregate(agg) => {
-                let op = token_display(agg.op.id());
-                write!(f, "{op}")?;
-                if let Some(modifier) = &agg.modifier {
-                    match modifier {
-                        LabelModifier::Include(labels) => {
-                            let labels = labels
-                                .iter()
-                                .map(|e| e.to_string())
-                                .collect::<Vec<String>>()
-                                .join(", ");
-                            if !labels.is_empty() {
-                                write!(f, " by ({labels}) ",)?;
-                            }
-                        }
-                        LabelModifier::Exclude(labels) => write!(
-                            f,
-                            " without ({}) ",
-                            labels
-                                .iter()
-                                .map(|e| e.to_string())
-                                .collect::<Vec<String>>()
-                                .join(", ")
-                        )?,
-                    }
-                }
-                write!(f, "(")?;
-                if let Some(param) = &agg.param {
-                    write!(f, "{param}, ")?;
-                }
-                let expr = agg.expr.as_ref();
-                write!(f, "{expr})")?;
-                Ok(())
-            }
+            Expr::Aggregate(agg) => write!(f, "{agg}"),
             Expr::Unary(UnaryExpr { expr }) => write!(f, "- {expr}"),
             Expr::Binary(binary) => {
                 let lhs = binary.lhs.as_ref();
@@ -1247,8 +1277,6 @@ fn check_ast_for_vector_selector(ex: VectorSelector) -> Result<Expr, String> {
 
 #[cfg(test)]
 mod tests {
-    use std::collections::HashSet;
-
     use super::*;
 
     #[test]
@@ -1322,35 +1350,27 @@ mod tests {
     #[test]
     fn test_binary_labels() {
         assert_eq!(
-            LabelModifier::Include(HashSet::from([String::from("foo"), String::from("bar")]))
-                .labels(),
-            &HashSet::from([String::from("foo"), String::from("bar")])
+            &vec![String::from("foo"), String::from("bar")],
+            LabelModifier::Include(vec![String::from("foo"), String::from("bar")]).labels()
         );
 
         assert_eq!(
-            LabelModifier::Exclude(HashSet::from([String::from("foo"), String::from("bar")]))
-                .labels(),
-            &HashSet::from([String::from("foo"), String::from("bar")])
+            &vec![String::from("foo"), String::from("bar")],
+            LabelModifier::Exclude(vec![String::from("foo"), String::from("bar")]).labels()
         );
 
         assert_eq!(
-            VectorMatchCardinality::OneToMany(HashSet::from([
-                String::from("foo"),
-                String::from("bar")
-            ]))
-            .labels()
-            .unwrap(),
-            &HashSet::from([String::from("foo"), String::from("bar")])
+            &vec![String::from("foo"), String::from("bar")],
+            VectorMatchCardinality::OneToMany(vec![String::from("foo"), String::from("bar")])
+                .labels()
+                .unwrap()
         );
 
         assert_eq!(
-            VectorMatchCardinality::ManyToOne(HashSet::from([
-                String::from("foo"),
-                String::from("bar")
-            ]))
-            .labels()
-            .unwrap(),
-            &HashSet::from([String::from("foo"), String::from("bar")])
+            &vec![String::from("foo"), String::from("bar")],
+            VectorMatchCardinality::ManyToOne(vec![String::from("foo"), String::from("bar")])
+                .labels()
+                .unwrap()
         );
 
         assert_eq!(VectorMatchCardinality::OneToOne.labels(), None);
diff --git a/src/parser/parse.rs b/src/parser/parse.rs
index de71cb9..4c8bcc9 100644
--- a/src/parser/parse.rs
+++ b/src/parser/parse.rs
@@ -413,13 +413,8 @@ mod tests {
                     token::T_DIV,
                     Some(
                         BinModifier::default()
-                            .with_card(VectorMatchCardinality::OneToMany(HashSet::from([
-                                String::from("test"),
-                            ])))
-                            .with_matching(Some(LabelModifier::Include(HashSet::from([
-                                String::from("baz"),
-                                String::from("buz"),
-                            ])))),
+                            .with_card(VectorMatchCardinality::one_to_many(vec!["test"]))
+                            .with_matching(Some(LabelModifier::include(vec!["baz", "buz"]))),
                     ),
                     Expr::from(VectorSelector::from("blub")),
                 )
@@ -428,9 +423,8 @@ mod tests {
                         Expr::from(VectorSelector::from("bar")),
                         token::T_ADD,
                         Some(
-                            BinModifier::default().with_matching(Some(LabelModifier::Include(
-                                HashSet::from([String::from("foo")]),
-                            ))),
+                            BinModifier::default()
+                                .with_matching(Some(LabelModifier::include(vec!["foo"]))),
                         ),
                         ex,
                     )
@@ -442,9 +436,8 @@ mod tests {
                     Expr::from(VectorSelector::from("foo")),
                     token::T_MUL,
                     Some(
-                        BinModifier::default().with_matching(Some(LabelModifier::Include(
-                            HashSet::from([String::from("test"), String::from("blub")]),
-                        ))),
+                        BinModifier::default()
+                            .with_matching(Some(LabelModifier::include(vec!["test", "blub"]))),
                     ),
                     Expr::from(VectorSelector::from("bar")),
                 ),
@@ -456,20 +449,14 @@ mod tests {
                     token::T_MUL,
                     Some(
                         BinModifier::default()
-                            .with_matching(Some(LabelModifier::Include(HashSet::from([
-                                String::from("test"),
-                                String::from("blub"),
-                            ]))))
-                            .with_card(VectorMatchCardinality::ManyToOne(HashSet::new())),
+                            .with_matching(Some(LabelModifier::include(vec!["test", "blub"])))
+                            .with_card(VectorMatchCardinality::many_to_one(vec![])),
                     ),
                     Expr::from(VectorSelector::from("bar")),
                 ),
             ),
             ("foo and on(test,blub) bar", {
-                let matching = LabelModifier::Include(HashSet::from([
-                    String::from("test"),
-                    String::from("blub"),
-                ]));
+                let matching = LabelModifier::include(vec!["test", "blub"]);
                 let card = VectorMatchCardinality::ManyToMany;
                 Expr::new_binary_expr(
                     Expr::from(VectorSelector::from("foo")),
@@ -483,7 +470,7 @@ mod tests {
                 )
             }),
             ("foo and on() bar", {
-                let matching = LabelModifier::Include(HashSet::new());
+                let matching = LabelModifier::include(vec![]);
                 let card = VectorMatchCardinality::ManyToMany;
                 Expr::new_binary_expr(
                     Expr::from(VectorSelector::from("foo")),
@@ -497,10 +484,7 @@ mod tests {
                 )
             }),
             ("foo and ignoring(test,blub) bar", {
-                let matching = LabelModifier::Exclude(HashSet::from([
-                    String::from("test"),
-                    String::from("blub"),
-                ]));
+                let matching = LabelModifier::exclude(vec!["test", "blub"]);
                 let card = VectorMatchCardinality::ManyToMany;
                 Expr::new_binary_expr(
                     Expr::from(VectorSelector::from("foo")),
@@ -514,7 +498,7 @@ mod tests {
                 )
             }),
             ("foo and ignoring() bar", {
-                let matching = LabelModifier::Exclude(HashSet::new());
+                let matching = LabelModifier::Exclude(vec![]);
                 let card = VectorMatchCardinality::ManyToMany;
                 Expr::new_binary_expr(
                     Expr::from(VectorSelector::from("foo")),
@@ -528,7 +512,7 @@ mod tests {
                 )
             }),
             ("foo unless on(bar) baz", {
-                let matching = LabelModifier::Include(HashSet::from([String::from("bar")]));
+                let matching = LabelModifier::include(vec!["bar"]);
                 let card = VectorMatchCardinality::ManyToMany;
                 Expr::new_binary_expr(
                     Expr::from(VectorSelector::from("foo")),
@@ -548,13 +532,8 @@ mod tests {
                     token::T_DIV,
                     Some(
                         BinModifier::default()
-                            .with_matching(Some(LabelModifier::Include(HashSet::from([
-                                String::from("test"),
-                                String::from("blub"),
-                            ]))))
-                            .with_card(VectorMatchCardinality::ManyToOne(HashSet::from([
-                                String::from("bar"),
-                            ]))),
+                            .with_matching(Some(LabelModifier::include(vec!["test", "blub"])))
+                            .with_card(VectorMatchCardinality::many_to_one(vec!["bar"])),
                     ),
                     Expr::from(VectorSelector::from("bar")),
                 ),
@@ -566,13 +545,8 @@ mod tests {
                     token::T_DIV,
                     Some(
                         BinModifier::default()
-                            .with_matching(Some(LabelModifier::Exclude(HashSet::from([
-                                String::from("test"),
-                                String::from("blub"),
-                            ]))))
-                            .with_card(VectorMatchCardinality::ManyToOne(HashSet::from([
-                                String::from("blub"),
-                            ]))),
+                            .with_matching(Some(LabelModifier::exclude(vec!["test", "blub"])))
+                            .with_card(VectorMatchCardinality::many_to_one(vec!["blub"])),
                     ),
                     Expr::from(VectorSelector::from("bar")),
                 ),
@@ -584,13 +558,8 @@ mod tests {
                     token::T_DIV,
                     Some(
                         BinModifier::default()
-                            .with_matching(Some(LabelModifier::Exclude(HashSet::from([
-                                String::from("test"),
-                                String::from("blub"),
-                            ]))))
-                            .with_card(VectorMatchCardinality::ManyToOne(HashSet::from([
-                                String::from("bar"),
-                            ]))),
+                            .with_matching(Some(LabelModifier::exclude(vec!["test", "blub"])))
+                            .with_card(VectorMatchCardinality::many_to_one(vec!["bar"])),
                     ),
                     Expr::from(VectorSelector::from("bar")),
                 ),
@@ -602,14 +571,8 @@ mod tests {
                     token::T_SUB,
                     Some(
                         BinModifier::default()
-                            .with_matching(Some(LabelModifier::Include(HashSet::from([
-                                String::from("test"),
-                                String::from("blub"),
-                            ]))))
-                            .with_card(VectorMatchCardinality::OneToMany(HashSet::from([
-                                String::from("bar"),
-                                String::from("foo"),
-                            ]))),
+                            .with_matching(Some(LabelModifier::include(vec!["test", "blub"])))
+                            .with_card(VectorMatchCardinality::one_to_many(vec!["bar", "foo"])),
                     ),
                     Expr::from(VectorSelector::from("bar")),
                 ),
@@ -621,14 +584,8 @@ mod tests {
                     token::T_SUB,
                     Some(
                         BinModifier::default()
-                            .with_matching(Some(LabelModifier::Exclude(HashSet::from([
-                                String::from("test"),
-                                String::from("blub"),
-                            ]))))
-                            .with_card(VectorMatchCardinality::OneToMany(HashSet::from([
-                                String::from("bar"),
-                                String::from("foo"),
-                            ]))),
+                            .with_matching(Some(LabelModifier::exclude(vec!["test", "blub"])))
+                            .with_card(VectorMatchCardinality::one_to_many(vec!["bar", "foo"])),
                     ),
                     Expr::from(VectorSelector::from("bar")),
                 ),
@@ -653,9 +610,8 @@ mod tests {
                         lhs,
                         token::T_DIV,
                         Some(
-                            BinModifier::default().with_matching(Some(LabelModifier::Exclude(
-                                HashSet::from([String::from("code")]),
-                            ))),
+                            BinModifier::default()
+                                .with_matching(Some(LabelModifier::exclude(vec!["code"]))),
                         ),
                         Expr::from(VectorSelector::from("method:http_requests:rate5m")),
                     )
@@ -668,10 +624,8 @@ mod tests {
                     token::T_DIV,
                     Some(
                         BinModifier::default()
-                            .with_matching(Some(LabelModifier::Exclude(HashSet::from([
-                                String::from("code"),
-                            ]))))
-                            .with_card(VectorMatchCardinality::ManyToOne(HashSet::new())),
+                            .with_matching(Some(LabelModifier::exclude(vec!["code"])))
+                            .with_card(VectorMatchCardinality::ManyToOne(vec![])),
                     ),
                     Expr::from(VectorSelector::from("method:http_requests:rate5m")),
                 ),
@@ -1161,26 +1115,26 @@ mod tests {
         let cases = vec![
             ("sum by (foo) (some_metric)", {
                 let ex = Expr::from(VectorSelector::from("some_metric"));
-                let modifier = LabelModifier::Include(HashSet::from([String::from("foo")]));
+                let modifier = LabelModifier::include(vec!["foo"]);
                 Expr::new_aggregate_expr(token::T_SUM, Some(modifier), FunctionArgs::new_args(ex))
             }),
             ("avg by (foo)(some_metric)", {
                 let ex = Expr::from(VectorSelector::from("some_metric"));
-                let modifier = LabelModifier::Include(HashSet::from([String::from("foo")]));
+                let modifier = LabelModifier::include(vec!["foo"]);
                 Expr::new_aggregate_expr(token::T_AVG, Some(modifier), FunctionArgs::new_args(ex))
             }),
             ("max by (foo)(some_metric)", {
-                let modifier = LabelModifier::Include(HashSet::from([String::from("foo")]));
+                let modifier = LabelModifier::include(vec!["foo"]);
                 let ex = Expr::from(VectorSelector::from("some_metric"));
                 Expr::new_aggregate_expr(token::T_MAX, Some(modifier), FunctionArgs::new_args(ex))
             }),
             ("sum without (foo) (some_metric)", {
-                let modifier = LabelModifier::Exclude(HashSet::from([String::from("foo")]));
+                let modifier = LabelModifier::exclude(vec!["foo"]);
                 let ex = Expr::from(VectorSelector::from("some_metric"));
                 Expr::new_aggregate_expr(token::T_SUM, Some(modifier), FunctionArgs::new_args(ex))
             }),
             ("sum (some_metric) without (foo)", {
-                let modifier = LabelModifier::Exclude(HashSet::from([String::from("foo")]));
+                let modifier = LabelModifier::exclude(vec!["foo"]);
                 let ex = Expr::from(VectorSelector::from("some_metric"));
                 Expr::new_aggregate_expr(token::T_SUM, Some(modifier), FunctionArgs::new_args(ex))
             }),
@@ -1189,7 +1143,7 @@ mod tests {
                 Expr::new_aggregate_expr(token::T_STDDEV, None, FunctionArgs::new_args(ex))
             }),
             ("stdvar by (foo)(some_metric)", {
-                let modifier = LabelModifier::Include(HashSet::from([String::from("foo")]));
+                let modifier = LabelModifier::include(vec!["foo"]);
                 let ex = Expr::from(VectorSelector::from("some_metric"));
                 Expr::new_aggregate_expr(
                     token::T_STDVAR,
@@ -1198,20 +1152,17 @@ mod tests {
                 )
             }),
             ("sum by ()(some_metric)", {
-                let modifier = LabelModifier::Include(HashSet::new());
+                let modifier = LabelModifier::Include(vec![]);
                 let ex = Expr::from(VectorSelector::from("some_metric"));
                 Expr::new_aggregate_expr(token::T_SUM, Some(modifier), FunctionArgs::new_args(ex))
             }),
             ("sum by (foo,bar,)(some_metric)", {
-                let modifier = LabelModifier::Include(HashSet::from([
-                    String::from("foo"),
-                    String::from("bar"),
-                ]));
+                let modifier = LabelModifier::include(vec!["foo", "bar"]);
                 let ex = Expr::from(VectorSelector::from("some_metric"));
                 Expr::new_aggregate_expr(token::T_SUM, Some(modifier), FunctionArgs::new_args(ex))
             }),
             ("sum by (foo,)(some_metric)", {
-                let modifier = LabelModifier::Include(HashSet::from([String::from("foo")]));
+                let modifier = LabelModifier::include(vec!["foo"]);
                 let ex = Expr::from(VectorSelector::from("some_metric"));
                 Expr::new_aggregate_expr(token::T_SUM, Some(modifier), FunctionArgs::new_args(ex))
             }),
@@ -1475,10 +1426,7 @@ mod tests {
                 .and_then(|ex| {
                     Expr::new_aggregate_expr(
                         token::T_SUM,
-                        Some(LabelModifier::Include(HashSet::from([
-                            String::from("job"),
-                            String::from("le"),
-                        ]))),
+                        Some(LabelModifier::include(vec!["job", "le"])),
                         FunctionArgs::new_args(ex),
                     )
                 })
@@ -2067,9 +2015,7 @@ mod tests {
             }),
             ("foo unless on(start) bar", {
                 let modifier = BinModifier::default()
-                    .with_matching(Some(LabelModifier::Include(HashSet::from([String::from(
-                        "start",
-                    )]))))
+                    .with_matching(Some(LabelModifier::include(vec!["start"])))
                     .with_card(VectorMatchCardinality::ManyToMany);
                 Expr::new_binary_expr(
                     Expr::from(VectorSelector::from("foo")),
@@ -2080,9 +2026,7 @@ mod tests {
             }),
             ("foo unless on(end) bar", {
                 let modifier = BinModifier::default()
-                    .with_matching(Some(LabelModifier::Include(HashSet::from([String::from(
-                        "end",
-                    )]))))
+                    .with_matching(Some(LabelModifier::include(vec!["end"])))
                     .with_card(VectorMatchCardinality::ManyToMany);
                 Expr::new_binary_expr(
                     Expr::from(VectorSelector::from("foo")),
@@ -2180,334 +2124,22 @@ mod tests {
                 "999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999",
                 "Inf"
             ),
-            // (
-            //     "\"double-quoted string \\\" with escaped quote\"",
-            //     "double-quoted string \\\" with escaped quote",
-            // ),
-            // (
-            //     r#""double-quoted string \" with escaped quote""#,
-            //     r#"double-quoted string \" with escaped quote"#,
-            // ),
-            // (
-            //     r#"'single-quoted string \' with escaped quote'"#,
-            //     r#"single-quoted string \' with escaped quote"#,
-            // ),
-            // (
-            //     "`backtick-quoted string`",
-            //     "backtick-quoted string",
-            // ),
-            // (
-            //     r#""\a\b\f\n\r\t\v\\\" - \xFF\377\u1234\U00010111\U0001011111☺""#,
-            //     r#"\a\b\f\n\r\t\v\\\" - \xFF\377\u1234\U00010111\U0001011111☺"#,
-            // ),
-            // (
-            //     r#"'\a\b\f\n\r\t\v\\\' - \xFF\377\u1234\U00010111\U0001011111☺'"#,
-            //     r#"\a\b\f\n\r\t\v\\\' - \xFF\377\u1234\U00010111\U0001011111☺"#,
-            // ),
-            // (
-            //     r#"`\a\b\f\n\r\t\v\\\` - \xFF\377\u1234\U00010111\U0001011111☺`"#,
-            //     r#"\a\b\f\n\r\t\v\\\` - \xFF\377\u1234\U00010111\U0001011111☺"#,
-            // ),
-            (
-                "1 + 1",
-                "1 + 1"
-            ),
-            (
-                "1 - 1",
-                "1 - 1"
-            ),
-            (
-                "1 * 1",
-                "1 * 1"
-            ),
-            (
-                "1 / 1",
-                "1 / 1"
-            ),
-            (
-                "1 % 1",
-                "1 % 1"
-            ),
-            (
-                "1 == bool 1",
-                "1 == bool 1",
-            ),
-            (
-                "1 != bool 1",
-                "1 != bool 1",
-            ),
-            (
-                "1 > bool 1",
-                "1 > bool 1",
-            ),
-            (
-                "1 >= bool 1",
-                "1 >= bool 1",
-            ),
-            (
-                "1 < bool 1",
-                "1 < bool 1",
-            ),
-            (
-                "1 <= bool 1",
-                "1 <= bool 1",
-            ),
-            (
-                "- 1 ^ 2",
-                "- 1 ^ 2",
-            ),
-            (
-                "-1*2",
-                "-1 * 2",
-            ),
-            (
-                "-1+2",
-                "-1 + 2",
-            ),
-            (
-                "-1^-2",
-                "- 1 ^ -2",
-            ),
-            (
-                "+1 + -2 * 1",
-                "1 + -2 * 1",
-            ),
-            (
-                "1 + 2/(3*1)",
-                "1 + 2 / (3 * 1)",
-            ),
-            (
-                "1 < bool 2 - 1 * 2",
-                "1 < bool 2 - 1 * 2"
-            ),
-            (
-                "foo * bar",
-                "foo * bar",
-            ),
-            (
-                "foo * sum",
-                "foo * sum",
-            ),
-            (
-                "foo == 1",
-                "foo == 1",
-            ),
-            (
-                "foo == bool 1",
-                "foo == bool 1",
-            ),
-            (
-                "2.5 / bar",
-                "2.5 / bar",
-            ),
-            (
-                "foo and bar",
-                "foo and bar",
-            ),
-            (
-                "foo or bar",
-                "foo or bar",
-            ),
-            (
-                "foo unless bar",
-                "foo unless bar",
-            ),
-            (
-                // Test and/or precedence and reassigning of operands.
-                "foo + bar or bla and blub",
-                "foo + bar or bla and blub",
-            ),
-            (
-                // Test and/or/unless precedence.
-                "foo and bar unless baz or qux",
-                "foo and bar unless baz or qux",
-            ),
+            ("- 1^2", "- 1 ^ 2"),
+            ("+1 + -2 * 1", "1 + -2 * 1"),
+            ("1 + 2/(3*1)", "1 + 2 / (3 * 1)"),
+            ("foo*sum", "foo * sum"),
+
             // (
             //     // Test precedence and reassigning of operands.
             //     "bar + on(foo) bla / on(baz, buz) group_right(test) blub",
             //     "bar + on (foo) bla / on (baz, buz) group_right (test) blub",
             // ),
-            // (
-            //     "foo * on(test,blub) bar",
-            //     "foo * on(test,blub) bar",
-            // ),
-            // (
-            //     "foo * on(test,blub) group_left bar",
-            //     "foo * on(test,blub) group_left bar",
-            // ),
-            // (
-            //     "foo and on(test,blub) bar",
-            //     "foo and on(test,blub) bar",
-            // ),
-            ("foo and on() bar", "foo and on () bar"),
-            // (
-            //     "foo and ignoring(test,blub) bar", 
-            //     "foo and ignoring(test,blub) bar"
-            // ),
-            ("foo and ignoring() bar", "foo and bar"),
-            ("foo unless on(bar) baz", "foo unless on (bar) baz"),
-            // (
-            //     "foo / on(test,blub) group_left(bar) bar",
-            //     "foo / on(test,blub) group_left(bar) bar",
-            // ),
-            // (
-            //     "foo / ignoring(test,blub) group_left(blub) bar",
-            //     "foo / ignoring(test,blub) group_left(blub) bar",
-            // ),
-            // (
-            //     "foo / ignoring(test,blub) group_left(bar) bar",
-            //     "foo / ignoring(test,blub) group_left(bar) bar",
-            // ),
-            // (
-            //     "foo - on(test,blub) group_right(bar,foo) bar",
-            //     "foo - on(test,blub) group_right(bar,foo) bar",
-            // ),
-            // (
-            //     "foo - ignoring(test,blub) group_right(bar,foo) bar",
-            //     "foo - ignoring(test,blub) group_right(bar,foo) bar",
-            // ),
-            (
-                "a + sum",
-                "a + sum",
-            ),
-            // cases from https://prometheus.io/docs/prometheus/latest/querying/operators
-            (
-                r#"method_code:http_errors:rate5m{code="500"} / ignoring(code) method:http_requests:rate5m"#,
-                r#"method_code:http_errors:rate5m{code="500"} / ignoring (code) method:http_requests:rate5m"#,
-            ),
-            (
-                r#"method_code:http_errors:rate5m / ignoring(code) group_left method:http_requests:rate5m"#,
-                r#"method_code:http_errors:rate5m / ignoring (code) group_left () method:http_requests:rate5m"#,
-            ),
-            (
-                r#"sum by() (task:errors:rate10s{job="s"})"#,
-                r#"sum(task:errors:rate10s{job="s"})"#
-            ),
-            (
-                r#"sum by(code) (task:errors:rate10s{job="s"})"#,
-                r#"sum by (code) (task:errors:rate10s{job="s"})"#
-            ),
-            (
-                r#"sum without() (task:errors:rate10s{job="s"})"#,
-                r#"sum without () (task:errors:rate10s{job="s"})"#
-            ),
-            (
-                r#"sum without(instance) (task:errors:rate10s{job="s"})"#,
-                r#"sum without (instance) (task:errors:rate10s{job="s"})"#
-            ),
-            (
-                r#"topk(5, task:errors:rate10s{job="s"})"#,
-                r#"topk(5, task:errors:rate10s{job="s"})"#
-            ),
-            (
-                r#"count_values("value", task:errors:rate10s{job="s"})"#,
-                r#"count_values("value", task:errors:rate10s{job="s"})"#
-            ),
-            (
-                "a - on() c",
-                "a - on () c",
-            ),
-            (
-                "a - on(b) c",
-                "a - on (b) c"
-            ),
-            (
-                "a - on(b) group_left(x) c",
-                "a - on (b) group_left (x) c"
-            ),
-            // (
-            //     "a - on(b) group_left(x, y) c",
-            //     "a - on (b) group_left (x, y) c"
-            // ),
-            (
-                "a - on(b) group_left c",
-                "a - on (b) group_left () c"
-            ),
-            (
-                "a - on(b) group_left() (c)",
-                "a - on (b) group_left () (c)"
-            ),
-            (
-                "a - ignoring(b) c",
-                "a - ignoring (b) c"
-            ),
-            (
-                "a - ignoring() c",
-                "a - c"
-            ),
-            (
-                "up > bool 0",
-                "up > bool 0"
-            ),
-            (
-                "a offset 1m",
-                "a offset 1m"
-            ),
-            (
-                "a offset -7m",
-                "a offset -7m"
-            ),
-            (
-                r#"a{c="d"}[5m] offset 1m"#,
-                r#"a{c="d"}[5m] offset 1m"#
-            ),
-            (
-                "a[5m] offset 1m",
-                "a[5m] offset 1m"
-            ),
-            (
-                "a[12m] offset -3m",
-                "a[12m] offset -3m"
-            ),
-            (
-                "a[1h:5m] offset 1m",
-                "a[1h:5m] offset 1m"
-            ),
-            // (
-            //     r#"{__name__="a"}"#,
-            //     r#"{__name__="a"}"#
-            // ),
-            (
-                r#"a{b!="c"}[1m]"#,
-                r#"a{b!="c"}[1m]"#
-            ),
-            (
-                r#"a{b=~"c"}[1m]"#,
-                r#"a{b=~"c"}[1m]"#
-            ),
-            (
-                r#"a{b!~"c"}[1m]"#,
-                r#"a{b!~"c"}[1m]"#
-            ),
-            // (
-            //     "a @ 10",
-            //     "a @ 10",
-            // ),
-            // (
-            //     "a[1m] @ 10",
-            //     "a[1m] @ 10"
-            // ),
-            (
-                "a @ start()",
-                "a @ start()"
-            ),
-            (
-                "a @ end()",
-                "a @ end()"
-            ),
-            (
-                "a[1m] @ start()",
-                "a[1m] @ start()"
-            ),
-            (
-                "a[1m] @ end()",
-                "a[1m] @ end()"
-            ),
+            // ("foo * on(test,blub) bar", "foo * on (test, blub) bar"),
+            ("foo * on(test,blub) bar", "foo * on (test, blub) bar"),
         ];
 
         for (input, expected) in cases {
-            let expr = crate::parser::parse(&String::from(input)).unwrap();
-            // println!("{expr:?}");
+            let expr = crate::parser::parse(input).unwrap();
             assert_eq!(expected, expr.to_string())
         }
     }
diff --git a/src/parser/promql.y b/src/parser/promql.y
index 8125ebb..04d5070 100644
--- a/src/parser/promql.y
+++ b/src/parser/promql.y
@@ -234,11 +234,11 @@ group_modifiers -> Result<Option<BinModifier>, String>:
                 }
         |       on_or_ignoring GROUP_LEFT
                 {
-                        Ok(update_optional_card($1?, VectorMatchCardinality::ManyToOne(HashSet::new())))
+                        Ok(update_optional_card($1?, VectorMatchCardinality::ManyToOne(vec![])))
                 }
         |       on_or_ignoring GROUP_RIGHT
                 {
-                        Ok(update_optional_card($1?, VectorMatchCardinality::OneToMany(HashSet::new())))
+                        Ok(update_optional_card($1?, VectorMatchCardinality::OneToMany(vec![])))
                 }
         |       GROUP_LEFT grouping_labels { Err("unexpected <group_left>".into()) }
         |       GROUP_RIGHT grouping_labels { Err("unexpected <group_right>".into()) }
@@ -247,17 +247,17 @@ group_modifiers -> Result<Option<BinModifier>, String>:
 grouping_labels -> Result<Labels, String>:
                 LEFT_PAREN grouping_label_list RIGHT_PAREN { $2 }
         |       LEFT_PAREN grouping_label_list COMMA RIGHT_PAREN { $2 }
-        |       LEFT_PAREN RIGHT_PAREN { Ok(HashSet::new()) }
+        |       LEFT_PAREN RIGHT_PAREN { Ok(vec![]) }
 ;
 
 grouping_label_list -> Result<Labels, String>:
                 grouping_label_list COMMA grouping_label
                 {
                         let mut v = $1?;
-                        v.insert($3?.val);
+                        v.push($3?.val);
                         Ok(v)
                 }
-        |       grouping_label { Ok(HashSet::from([$1?.val])) }
+        |       grouping_label { Ok(vec![$1?.val]) }
 ;
 
 grouping_label -> Result<Token, String>:
@@ -570,7 +570,6 @@ maybe_duration -> Result<Option<Duration>, String>:
 
 %%
 
-use std::collections::HashSet;
 use std::time::Duration;
 use crate::label::{Labels, Matcher, Matchers};
 use crate::parser::{