From 51100a542ce7d0bdfb1b1971b22f2b5cf875a49d Mon Sep 17 00:00:00 2001
From: Ning Sun <sunning@greptime.com>
Date: Wed, 16 Oct 2024 16:01:56 -0700
Subject: [PATCH 1/8] feat: serialize AST to json

---
 Cargo.toml             |  8 +++++++
 src/label/matcher.rs   | 19 +++++++++++++++++
 src/label/mod.rs       |  1 +
 src/parser/ast.rs      | 30 +++++++++++++++++++++++++++
 src/parser/function.rs |  2 ++
 src/parser/token.rs    |  1 +
 src/parser/value.rs    |  1 +
 tests/ser.rs           | 47 ++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 109 insertions(+)
 create mode 100644 tests/ser.rs

diff --git a/Cargo.toml b/Cargo.toml
index 3d11101..52a23e3 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -31,6 +31,14 @@ lazy_static = "1.4.0"
 lrlex = "0.13.5"
 lrpar = "0.13.5"
 regex = "1"
+serde = { version = "1", optional = true }
+
+[features]
+default = []
+ser = ["serde"]
+
+[dev-dependencies]
+serde_json = "1"
 
 [build-dependencies]
 cfgrammar = "0.13.5"
diff --git a/src/label/matcher.rs b/src/label/matcher.rs
index 2e009f4..0f19f54 100644
--- a/src/label/matcher.rs
+++ b/src/label/matcher.rs
@@ -24,6 +24,7 @@ use crate::util::join_vector;
 pub enum MatchOp {
     Equal,
     NotEqual,
+    // TODO: do we need regex here?
     Re(Regex),
     NotRe(Regex),
 }
@@ -64,9 +65,26 @@ impl Hash for MatchOp {
     }
 }
 
+#[cfg(feature = "ser")]
+impl serde::Serialize for MatchOp {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        match self {
+            MatchOp::Equal => serializer.serialize_str("="),
+            MatchOp::NotEqual => serializer.serialize_str("=~"),
+            MatchOp::Re(_reg) => serializer.serialize_str("=~"),
+            MatchOp::NotRe(_reg) => serializer.serialize_str("!~"),
+        }
+    }
+}
+
 // Matcher models the matching of a label.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct Matcher {
+    #[cfg_attr(feature = "ser", serde(rename = "type"))]
     pub op: MatchOp,
     pub name: String,
     pub value: String,
@@ -183,6 +201,7 @@ fn try_escape_for_repeat_re(re: &str) -> String {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct Matchers {
     pub matchers: Vec<Matcher>,
     pub or_matchers: Vec<Vec<Matcher>>,
diff --git a/src/label/mod.rs b/src/label/mod.rs
index 1bba6d2..96ea999 100644
--- a/src/label/mod.rs
+++ b/src/label/mod.rs
@@ -32,6 +32,7 @@ pub const INSTANCE_NAME: &str = "instance";
 pub type Label = String;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct Labels {
     pub labels: Vec<Label>,
 }
diff --git a/src/parser/ast.rs b/src/parser/ast.rs
index 2131388..55c2a4d 100644
--- a/src/parser/ast.rs
+++ b/src/parser/ast.rs
@@ -43,6 +43,7 @@ use std::time::{Duration, SystemTime};
 ///
 /// if empty listed labels, meaning no grouping
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub enum LabelModifier {
     Include(Labels),
     Exclude(Labels),
@@ -72,6 +73,7 @@ impl LabelModifier {
 /// The label list provided with the group_left or group_right modifier contains
 /// additional labels from the "one"-side to be included in the result metrics.
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub enum VectorMatchCardinality {
     OneToOne,
     ManyToOne(Labels),
@@ -100,6 +102,7 @@ impl VectorMatchCardinality {
 
 /// Binary Expr Modifier
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct BinModifier {
     /// The matching behavior for the operation if both operands are Vectors.
     /// If they are not this field is None.
@@ -211,7 +214,19 @@ impl fmt::Display for Offset {
         }
     }
 }
+
+#[cfg(feature = "ser")]
+impl serde::Serialize for Offset {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serializer.serialize_str(&format!("{}", self))
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub enum AtModifier {
     Start,
     End,
@@ -339,6 +354,7 @@ impl fmt::Display for EvalStmt {
 ///
 /// parameter is only required for `count_values`, `quantile`, `topk` and `bottomk`.
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct AggregateExpr {
     /// The used aggregation operation.
     pub op: TokenType,
@@ -393,6 +409,7 @@ impl Prettier for AggregateExpr {
 
 /// UnaryExpr will negate the expr
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct UnaryExpr {
     pub expr: Box<Expr>,
 }
@@ -421,6 +438,7 @@ impl Prettier for UnaryExpr {
 /// <vector expr> <bin-op> on(<label list>) group_right(<label list>) <vector expr>
 /// ```
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct BinaryExpr {
     /// The operation of the expression.
     pub op: TokenType,
@@ -490,6 +508,7 @@ impl Prettier for BinaryExpr {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct ParenExpr {
     pub expr: Box<Expr>,
 }
@@ -516,6 +535,7 @@ impl Prettier for ParenExpr {
 /// <instant_query> '[' <range> ':' [<resolution>] ']' [ @ <float_literal> ] [ offset <duration> ]
 /// ```
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct SubqueryExpr {
     pub expr: Box<Expr>,
     pub offset: Option<Offset>,
@@ -563,6 +583,7 @@ impl Prettier for SubqueryExpr {
 }
 
 #[derive(Debug, Clone)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct NumberLiteral {
     pub val: f64,
 }
@@ -610,6 +631,7 @@ impl Prettier for NumberLiteral {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct StringLiteral {
     pub val: String,
 }
@@ -627,10 +649,13 @@ impl Prettier for StringLiteral {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct VectorSelector {
     pub name: Option<String>,
+    #[cfg_attr(feature = "ser", serde(flatten))]
     pub matchers: Matchers,
     pub offset: Option<Offset>,
+    // TODO: special handling for ser
     pub at: Option<AtModifier>,
 }
 
@@ -727,6 +752,7 @@ impl Prettier for VectorSelector {
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct MatrixSelector {
     pub vs: VectorSelector,
     pub range: Duration,
@@ -803,6 +829,7 @@ impl Prettier for MatrixSelector {
 ///  - tan()
 ///  - tanh()
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct Call {
     pub func: Function,
     pub args: FunctionArgs,
@@ -852,6 +879,8 @@ impl PartialEq for Extension {
 impl Eq for Extension {}
 
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
+#[cfg_attr(feature = "ser", serde(tag = "type", rename_all = "camelCase"))]
 pub enum Expr {
     /// Aggregate represents an aggregation operation on a Vector.
     Aggregate(AggregateExpr),
@@ -887,6 +916,7 @@ pub enum Expr {
 
     /// Extension represents an extension expression. It is for user to attach additional
     /// information to the AST. This parser won't generate Extension node.
+    #[cfg_attr(feature = "ser", serde(skip))]
     Extension(Extension),
 }
 
diff --git a/src/parser/function.rs b/src/parser/function.rs
index 7565a90..81bc2aa 100644
--- a/src/parser/function.rs
+++ b/src/parser/function.rs
@@ -23,6 +23,7 @@ use crate::util::join_vector;
 
 /// called by func in Call
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct FunctionArgs {
     pub args: Vec<Box<Expr>>,
 }
@@ -78,6 +79,7 @@ impl Prettier for FunctionArgs {
 
 /// Functions is a list of all functions supported by PromQL, including their types.
 #[derive(Debug, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct Function {
     pub name: &'static str,
     pub arg_types: Vec<ValueType>,
diff --git a/src/parser/token.rs b/src/parser/token.rs
index 4f2ee52..43c53cd 100644
--- a/src/parser/token.rs
+++ b/src/parser/token.rs
@@ -22,6 +22,7 @@ pub use token_map::*;
 pub type TokenId = u8;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct TokenType(TokenId);
 
 lazy_static! {
diff --git a/src/parser/value.rs b/src/parser/value.rs
index 3186cb5..bd302f8 100644
--- a/src/parser/value.rs
+++ b/src/parser/value.rs
@@ -15,6 +15,7 @@
 use std::fmt::{self, Display};
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub enum ValueType {
     Vector,
     Scalar,
diff --git a/tests/ser.rs b/tests/ser.rs
new file mode 100644
index 0000000..2c4ab39
--- /dev/null
+++ b/tests/ser.rs
@@ -0,0 +1,47 @@
+// Copyright 2023 Greptime Team
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+#![cfg(feature = "ser")]
+
+use promql_parser::parser::parse;
+
+macro_rules! assert_json_ser_eq {
+    ($promql: literal, $json: tt) => {
+        let ast = parse($promql).expect("Failed to parse");
+        assert_eq!(
+            serde_json::json!($json),
+            serde_json::to_value(ast).expect("Failed to serialize")
+        );
+    };
+}
+
+#[test]
+fn test_serialize() {
+    assert_json_ser_eq!(
+        "prometheus_tsdb_wal_writes_failed_total",
+
+    {
+    "matchers": [
+      {
+        "name": "__name__",
+        "type": "=",
+        "value": "prometheus_tsdb_wal_writes_failed_total"
+      }
+    ],
+    "name": "prometheus_tsdb_wal_writes_failed_total",
+    "offset": 0,
+    "startOrEnd": null,
+    "timestamp": null,
+    "type": "vectorSelector"
+    });
+}

From b18516a0dd7e1ba30581e2c66f9759ccf2450bda Mon Sep 17 00:00:00 2001
From: Ning Sun <sunning@greptime.com>
Date: Wed, 16 Oct 2024 16:06:06 -0700
Subject: [PATCH 2/8] chore: make header check happy again

---
 tests/ser.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/ser.rs b/tests/ser.rs
index 2c4ab39..518b9e4 100644
--- a/tests/ser.rs
+++ b/tests/ser.rs
@@ -11,6 +11,7 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
+
 #![cfg(feature = "ser")]
 
 use promql_parser::parser::parse;

From 8abcbd96e540dcf60a01a0512131306899a254ae Mon Sep 17 00:00:00 2001
From: Ning Sun <sunning@greptime.com>
Date: Wed, 16 Oct 2024 22:01:01 -0700
Subject: [PATCH 3/8] feat: implement serialize for vectorSelector and call

---
 src/label/matcher.rs   |  1 +
 src/parser/ast.rs      | 72 +++++++++++++++++++++++++++++++++++-------
 src/parser/function.rs | 17 ++++++++++
 src/parser/value.rs    |  1 +
 src/util/duration.rs   |  9 ++++++
 tests/ser.rs           | 60 ++++++++++++++++++++++++++++-------
 6 files changed, 137 insertions(+), 23 deletions(-)

diff --git a/src/label/matcher.rs b/src/label/matcher.rs
index 0f19f54..9d05bae 100644
--- a/src/label/matcher.rs
+++ b/src/label/matcher.rs
@@ -204,6 +204,7 @@ fn try_escape_for_repeat_re(re: &str) -> String {
 #[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct Matchers {
     pub matchers: Vec<Matcher>,
+    #[cfg_attr(feature = "ser", serde(skip_serializing_if = "<[_]>::is_empty"))]
     pub or_matchers: Vec<Vec<Matcher>>,
 }
 
diff --git a/src/parser/ast.rs b/src/parser/ast.rs
index 55c2a4d..2f46841 100644
--- a/src/parser/ast.rs
+++ b/src/parser/ast.rs
@@ -206,27 +206,34 @@ pub enum Offset {
     Neg(Duration),
 }
 
-impl fmt::Display for Offset {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+impl Offset {
+    pub fn value_millis(&self) -> i128 {
         match self {
-            Offset::Pos(dur) => write!(f, "{}", display_duration(dur)),
-            Offset::Neg(dur) => write!(f, "-{}", display_duration(dur)),
+            Self::Pos(dur) => dur.as_millis() as i128,
+            Self::Neg(dur) => -(dur.as_millis() as i128),
         }
     }
-}
 
-#[cfg(feature = "ser")]
-impl serde::Serialize for Offset {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    #[cfg(feature = "ser")]
+    pub fn serialize_offset<S>(offset: &Option<Self>, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
     {
-        serializer.serialize_str(&format!("{}", self))
+        let value = offset.as_ref().map(|o| o.value_millis()).unwrap_or(0);
+        serializer.serialize_i128(value)
+    }
+}
+
+impl fmt::Display for Offset {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Offset::Pos(dur) => write!(f, "{}", display_duration(dur)),
+            Offset::Neg(dur) => write!(f, "-{}", display_duration(dur)),
+        }
     }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
-#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub enum AtModifier {
     Start,
     End,
@@ -248,6 +255,40 @@ impl fmt::Display for AtModifier {
         }
     }
 }
+
+#[cfg(feature = "ser")]
+impl serde::Serialize for AtModifier {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        use serde::ser::SerializeMap;
+        let mut map = serializer.serialize_map(Some(2))?;
+        match self {
+            AtModifier::Start => {
+                map.serialize_entry("startOrEnd", &Some("start"))?;
+                map.serialize_entry("timestamp", &None::<u128>)?;
+            }
+            AtModifier::End => {
+                map.serialize_entry("startOrEnd", &Some("end"))?;
+                map.serialize_entry("timestamp", &None::<u128>)?;
+            }
+            AtModifier::At(time) => {
+                map.serialize_entry("startOrEnd", &None::<&str>)?;
+                map.serialize_entry(
+                    "timestamp",
+                    &time
+                        .duration_since(SystemTime::UNIX_EPOCH)
+                        .unwrap_or(Duration::ZERO)
+                        .as_millis(),
+                )?;
+            }
+        }
+
+        map.end()
+    }
+}
+
 impl TryFrom<TokenId> for AtModifier {
     type Error = String;
 
@@ -538,7 +579,9 @@ impl Prettier for ParenExpr {
 #[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct SubqueryExpr {
     pub expr: Box<Expr>,
+    #[cfg_attr(feature = "ser", serde(serialize_with = "Offset::serialize_offset"))]
     pub offset: Option<Offset>,
+    #[cfg_attr(feature = "ser", serde(flatten))]
     pub at: Option<AtModifier>,
     pub range: Duration,
     /// Default is the global evaluation interval.
@@ -654,8 +697,9 @@ pub struct VectorSelector {
     pub name: Option<String>,
     #[cfg_attr(feature = "ser", serde(flatten))]
     pub matchers: Matchers,
+    #[cfg_attr(feature = "ser", serde(serialize_with = "Offset::serialize_offset"))]
     pub offset: Option<Offset>,
-    // TODO: special handling for ser
+    #[cfg_attr(feature = "ser", serde(flatten))]
     pub at: Option<AtModifier>,
 }
 
@@ -754,7 +798,12 @@ impl Prettier for VectorSelector {
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct MatrixSelector {
+    #[cfg_attr(feature = "ser", serde(flatten))]
     pub vs: VectorSelector,
+    #[cfg_attr(
+        feature = "ser",
+        serde(serialize_with = "crate::util::duration::serialize_duration")
+    )]
     pub range: Duration,
 }
 
@@ -832,6 +881,7 @@ impl Prettier for MatrixSelector {
 #[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct Call {
     pub func: Function,
+    #[cfg_attr(feature = "ser", serde(flatten))]
     pub args: FunctionArgs,
 }
 
diff --git a/src/parser/function.rs b/src/parser/function.rs
index 81bc2aa..c05c42e 100644
--- a/src/parser/function.rs
+++ b/src/parser/function.rs
@@ -80,9 +80,14 @@ impl Prettier for FunctionArgs {
 /// Functions is a list of all functions supported by PromQL, including their types.
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "ser", derive(serde::Serialize))]
+#[cfg_attr(feature = "ser", serde(rename_all = "camelCase"))]
 pub struct Function {
     pub name: &'static str,
     pub arg_types: Vec<ValueType>,
+    #[cfg_attr(
+        feature = "ser",
+        serde(serialize_with = "Function::serialize_variadic")
+    )]
     pub variadic: bool,
     pub return_type: ValueType,
 }
@@ -101,6 +106,18 @@ impl Function {
             return_type,
         }
     }
+
+    #[cfg(feature = "ser")]
+    pub fn serialize_variadic<S>(variadic: &bool, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        if *variadic {
+            serializer.serialize_i8(1)
+        } else {
+            serializer.serialize_i8(0)
+        }
+    }
 }
 
 macro_rules! map {
diff --git a/src/parser/value.rs b/src/parser/value.rs
index bd302f8..5a74fc0 100644
--- a/src/parser/value.rs
+++ b/src/parser/value.rs
@@ -16,6 +16,7 @@ use std::fmt::{self, Display};
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[cfg_attr(feature = "ser", derive(serde::Serialize))]
+#[cfg_attr(feature = "ser", serde(rename_all = "camelCase"))]
 pub enum ValueType {
     Vector,
     Scalar,
diff --git a/src/util/duration.rs b/src/util/duration.rs
index 32d9f17..2621e1e 100644
--- a/src/util/duration.rs
+++ b/src/util/duration.rs
@@ -135,6 +135,15 @@ pub fn display_duration(duration: &Duration) -> String {
     ss
 }
 
+#[cfg(feature = "ser")]
+pub fn serialize_duration<S>(dur: &Duration, serializer: S) -> Result<S::Ok, S::Error>
+where
+    S: serde::Serializer,
+{
+    let duration_millis = dur.as_millis();
+    serializer.serialize_u128(duration_millis)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/tests/ser.rs b/tests/ser.rs
index 518b9e4..704094c 100644
--- a/tests/ser.rs
+++ b/tests/ser.rs
@@ -32,17 +32,53 @@ fn test_serialize() {
         "prometheus_tsdb_wal_writes_failed_total",
 
     {
-    "matchers": [
-      {
-        "name": "__name__",
-        "type": "=",
-        "value": "prometheus_tsdb_wal_writes_failed_total"
-      }
-    ],
-    "name": "prometheus_tsdb_wal_writes_failed_total",
-    "offset": 0,
-    "startOrEnd": null,
-    "timestamp": null,
-    "type": "vectorSelector"
+        "matchers": [],
+        "name": "prometheus_tsdb_wal_writes_failed_total",
+        "offset": 0,
+        "type": "vectorSelector"
     });
+
+    assert_json_ser_eq!(
+        "prometheus_tsdb_wal_writes_failed_total offset 2s @ start()",
+
+    {
+        "matchers": [],
+        "name": "prometheus_tsdb_wal_writes_failed_total",
+        "offset": 2000,
+        "startOrEnd": "start",
+        "timestamp": null,
+        "type": "vectorSelector"
+    });
+
+    assert_json_ser_eq!(
+            "rate(prometheus_tsdb_wal_writes_failed_total{instance = \"localhost:9090\"}[1m])",
+
+    {
+        "args": [
+            {
+                "matchers": [
+                    {
+                        "name": "instance",
+                        "type": "=",
+                        "value": "localhost:9090"
+                    }
+                ],
+                "name": "prometheus_tsdb_wal_writes_failed_total",
+                "offset": 0,
+                "range": 60000,
+                "type": "matrixSelector"
+            }
+        ],
+        "func": {
+            "argTypes": [
+                "matrix"
+            ],
+            "name": "rate",
+            "returnType": "vector",
+            "variadic": 0
+        },
+        "type": "call"
+    }
+
+    );
 }

From 5ab3367e86f9b1e23e88b4f318a3675b431c6cbb Mon Sep 17 00:00:00 2001
From: Ning Sun <sunning@greptime.com>
Date: Wed, 16 Oct 2024 22:06:24 -0700
Subject: [PATCH 4/8] test: improve converage

---
 tests/ser.rs | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tests/ser.rs b/tests/ser.rs
index 704094c..c0aa593 100644
--- a/tests/ser.rs
+++ b/tests/ser.rs
@@ -50,6 +50,30 @@ fn test_serialize() {
         "type": "vectorSelector"
     });
 
+    assert_json_ser_eq!(
+        "prometheus_tsdb_wal_writes_failed_total offset -2s @ end()",
+
+    {
+        "matchers": [],
+        "name": "prometheus_tsdb_wal_writes_failed_total",
+        "offset": -2000,
+        "startOrEnd": "end",
+        "timestamp": null,
+        "type": "vectorSelector"
+    });
+
+    assert_json_ser_eq!(
+        "prometheus_tsdb_wal_writes_failed_total @ 1000",
+
+    {
+        "matchers": [],
+        "name": "prometheus_tsdb_wal_writes_failed_total",
+        "offset": 0,
+        "startOrEnd": null,
+        "timestamp": 1000000,
+        "type": "vectorSelector"
+    });
+
     assert_json_ser_eq!(
             "rate(prometheus_tsdb_wal_writes_failed_total{instance = \"localhost:9090\"}[1m])",
 

From 16336edb7c18a0378026948a7a1a93e2f02e3163 Mon Sep 17 00:00:00 2001
From: Ning Sun <sunning@greptime.com>
Date: Thu, 17 Oct 2024 15:35:31 -0700
Subject: [PATCH 5/8] feat: implement serialization for more

---
 Cargo.toml          |   6 +-
 src/label/mod.rs    |  18 ++++-
 src/parser/ast.rs   |  98 +++++++++++++++++++++++-
 src/parser/token.rs |  11 ++-
 tests/ser.rs        | 177 +++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 298 insertions(+), 12 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 52a23e3..64b327b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -32,13 +32,11 @@ lrlex = "0.13.5"
 lrpar = "0.13.5"
 regex = "1"
 serde = { version = "1", optional = true }
+serde_json = { version = "1", optional = true }
 
 [features]
 default = []
-ser = ["serde"]
-
-[dev-dependencies]
-serde_json = "1"
+ser = ["serde", "serde_json"]
 
 [build-dependencies]
 cfgrammar = "0.13.5"
diff --git a/src/label/mod.rs b/src/label/mod.rs
index 96ea999..8549816 100644
--- a/src/label/mod.rs
+++ b/src/label/mod.rs
@@ -32,7 +32,6 @@ pub const INSTANCE_NAME: &str = "instance";
 pub type Label = String;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
-#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct Labels {
     pub labels: Vec<Label>,
 }
@@ -74,6 +73,23 @@ impl fmt::Display for Labels {
     }
 }
 
+#[cfg(feature = "ser")]
+impl serde::Serialize for Labels {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        use serde::ser::SerializeSeq;
+        let mut seq = serializer.serialize_seq(Some(self.labels.len()))?;
+
+        for l in &self.labels {
+            seq.serialize_element(&l)?;
+        }
+
+        seq.end()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/src/parser/ast.rs b/src/parser/ast.rs
index 2f46841..8a57e95 100644
--- a/src/parser/ast.rs
+++ b/src/parser/ast.rs
@@ -74,6 +74,7 @@ impl LabelModifier {
 /// additional labels from the "one"-side to be included in the result metrics.
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[cfg_attr(feature = "ser", derive(serde::Serialize))]
+#[cfg_attr(feature = "ser", serde(rename_all = "kebab-case"))]
 pub enum VectorMatchCardinality {
     OneToOne,
     ManyToOne(Labels),
@@ -102,7 +103,6 @@ impl VectorMatchCardinality {
 
 /// Binary Expr Modifier
 #[derive(Debug, Clone, PartialEq, Eq)]
-#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct BinModifier {
     /// The matching behavior for the operation if both operands are Vectors.
     /// If they are not this field is None.
@@ -200,6 +200,63 @@ impl BinModifier {
     }
 }
 
+#[cfg(feature = "ser")]
+pub(crate) fn serialize_bin_modifier<S>(
+    this: &Option<BinModifier>,
+    serializer: S,
+) -> Result<S::Ok, S::Error>
+where
+    S: serde::Serializer,
+{
+    use serde::ser::SerializeMap;
+    use serde_json::json;
+
+    let mut map = serializer.serialize_map(Some(2))?;
+
+    map.serialize_entry(
+        "bool",
+        &this.as_ref().map(|t| t.return_bool).unwrap_or(false),
+    )?;
+    if let Some(t) = this {
+        if let Some(labels) = &t.matching {
+            map.serialize_key("matching")?;
+
+            match labels {
+                LabelModifier::Include(labels) => {
+                    let value = json!({
+                        "card": t.card,
+                        "include": [],
+                        "labels": labels,
+                        "on": true,
+                    });
+                    map.serialize_value(&value)?;
+                }
+                LabelModifier::Exclude(labels) => {
+                    let value = json!({
+                        "card": t.card,
+                        "include": [],
+                        "labels": labels,
+                        "on": false,
+                    });
+                    map.serialize_value(&value)?;
+                }
+            }
+        } else {
+            let value = json!({
+                "card": t.card,
+                "include": [],
+                "labels": [],
+                "on": false,
+            });
+            map.serialize_entry("matching", &value)?;
+        }
+    } else {
+        map.serialize_entry("matching", &None::<bool>)?;
+    }
+
+    map.end()
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum Offset {
     Pos(Duration),
@@ -450,7 +507,6 @@ impl Prettier for AggregateExpr {
 
 /// UnaryExpr will negate the expr
 #[derive(Debug, Clone, PartialEq, Eq)]
-#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct UnaryExpr {
     pub expr: Box<Expr>,
 }
@@ -471,6 +527,21 @@ impl Prettier for UnaryExpr {
     }
 }
 
+#[cfg(feature = "ser")]
+impl serde::Serialize for UnaryExpr {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        use serde::ser::SerializeMap;
+        let mut map = serializer.serialize_map(Some(2))?;
+        map.serialize_entry("op", "-")?;
+        map.serialize_entry("expr", &self.expr)?;
+
+        map.end()
+    }
+}
+
 /// Grammar:
 /// ``` norust
 /// <vector expr> <bin-op> ignoring(<label list>) group_left(<label list>) <vector expr>
@@ -487,7 +558,8 @@ pub struct BinaryExpr {
     pub lhs: Box<Expr>,
     /// The operands on the right sides of the operator.
     pub rhs: Box<Expr>,
-
+    #[cfg_attr(feature = "ser", serde(flatten))]
+    #[cfg_attr(feature = "ser", serde(serialize_with = "serialize_bin_modifier"))]
     pub modifier: Option<BinModifier>,
 }
 
@@ -626,7 +698,6 @@ impl Prettier for SubqueryExpr {
 }
 
 #[derive(Debug, Clone)]
-#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct NumberLiteral {
     pub val: f64,
 }
@@ -667,6 +738,20 @@ impl fmt::Display for NumberLiteral {
     }
 }
 
+#[cfg(feature = "ser")]
+impl serde::Serialize for NumberLiteral {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        use serde::ser::SerializeMap;
+        let mut map = serializer.serialize_map(Some(1))?;
+        map.serialize_entry("val", &self.to_string())?;
+
+        map.end()
+    }
+}
+
 impl Prettier for NumberLiteral {
     fn needs_split(&self, _max: usize) -> bool {
         false
@@ -933,20 +1018,25 @@ impl Eq for Extension {}
 #[cfg_attr(feature = "ser", serde(tag = "type", rename_all = "camelCase"))]
 pub enum Expr {
     /// Aggregate represents an aggregation operation on a Vector.
+    #[cfg_attr(feature = "ser", serde(rename = "aggregateExpr"))]
     Aggregate(AggregateExpr),
 
     /// Unary represents a unary operation on another expression.
     /// Currently unary operations are only supported for Scalars.
+    #[cfg_attr(feature = "ser", serde(rename = "unaryExpr"))]
     Unary(UnaryExpr),
 
     /// Binary represents a binary expression between two child expressions.
+    #[cfg_attr(feature = "ser", serde(rename = "binaryExpr"))]
     Binary(BinaryExpr),
 
     /// Paren wraps an expression so it cannot be disassembled as a consequence
     /// of operator precedence.
+    #[cfg_attr(feature = "ser", serde(rename = "parenExpr"))]
     Paren(ParenExpr),
 
     /// SubqueryExpr represents a subquery.
+    #[cfg_attr(feature = "ser", serde(rename = "subqueryExpr"))]
     Subquery(SubqueryExpr),
 
     /// NumberLiteral represents a number.
diff --git a/src/parser/token.rs b/src/parser/token.rs
index 43c53cd..6696bbc 100644
--- a/src/parser/token.rs
+++ b/src/parser/token.rs
@@ -22,9 +22,18 @@ pub use token_map::*;
 pub type TokenId = u8;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
-#[cfg_attr(feature = "ser", derive(serde::Serialize))]
 pub struct TokenType(TokenId);
 
+#[cfg(feature = "ser")]
+impl serde::Serialize for TokenType {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serializer.serialize_str(token_display(self.0))
+    }
+}
+
 lazy_static! {
     static ref KEYWORDS: HashMap<&'static str, TokenId> =
         [
diff --git a/tests/ser.rs b/tests/ser.rs
index c0aa593..0ae3187 100644
--- a/tests/ser.rs
+++ b/tests/ser.rs
@@ -20,8 +20,8 @@ macro_rules! assert_json_ser_eq {
     ($promql: literal, $json: tt) => {
         let ast = parse($promql).expect("Failed to parse");
         assert_eq!(
-            serde_json::json!($json),
-            serde_json::to_value(ast).expect("Failed to serialize")
+            serde_json::to_value(ast).expect("Failed to serialize"),
+            serde_json::json!($json)
         );
     };
 }
@@ -105,4 +105,177 @@ fn test_serialize() {
     }
 
     );
+
+    assert_json_ser_eq!("\"yes\"",
+    {
+        "type": "stringLiteral",
+        "val": "yes"
+    }
+    );
+
+    assert_json_ser_eq!("1",
+    {
+        "type": "numberLiteral",
+        "val": "1"
+    }
+    );
+
+    assert_json_ser_eq!("1+1",
+    {
+        "bool": false,
+        "lhs": {
+            "type": "numberLiteral",
+            "val": "1"
+        },
+        "op": "+",
+        "matching": null,
+        "rhs": {
+            "type": "numberLiteral",
+            "val": "1"
+        },
+        "type": "binaryExpr"
+    }
+    );
+
+    assert_json_ser_eq!("- process_cpu_seconds_total",
+        {
+            "expr": {
+                "matchers": [],
+                "name": "process_cpu_seconds_total",
+                "offset": 0,
+                "type": "vectorSelector"
+            },
+            "op": "-",
+            "type": "unaryExpr"
+        }
+    );
+
+    assert_json_ser_eq!(r#"1 - ((node_memory_MemAvailable_bytes{job="node"} or (node_memory_Buffers_bytes{job="node"} + node_memory_Cached_bytes{job="node"} + node_memory_MemFree_bytes{job="node"} + node_memory_Slab_bytes{job="node"}) ) / node_memory_MemTotal_bytes{job="node"})"#,
+        {
+            "bool": false,
+            "lhs": {
+                "type": "numberLiteral",
+                "val": "1"
+            },
+            "op": "-",
+            "matching": null,
+            "rhs": {
+                "expr": {
+                    "bool": false,
+                    "lhs": {
+                        "expr": {
+                            "bool": false,
+                            "lhs": {
+                                "matchers": [
+                                    {
+                                        "name": "job",
+                                        "type": "=",
+                                        "value": "node"
+                                    }
+                                ],
+                                "name": "node_memory_MemAvailable_bytes",
+                                "offset": 0,
+                                "type": "vectorSelector"
+                            },
+                            "matching": {
+                                "card": "many-to-many",
+                                "include": [],
+                                "labels": [],
+                                "on": false
+                            },
+                            "op": "or",
+                            "rhs": {
+                                "expr": {
+                                    "bool": false,
+                                    "lhs": {
+                                        "bool": false,
+                                        "lhs": {
+                                            "bool": false,
+                                            "lhs": {
+                                                "matchers": [
+                                                    {
+                                                        "name": "job",
+                                                        "type": "=",
+                                                        "value": "node"
+                                                    }
+                                                ],
+                                                "name": "node_memory_Buffers_bytes",
+                                                "offset": 0,
+                                                "type": "vectorSelector"
+                                            },
+                                            "matching": null,
+                                            "op": "+",
+                                            "rhs": {
+                                                "matchers": [
+                                                    {
+                                                        "name": "job",
+                                                        "type": "=",
+                                                        "value": "node"
+                                                    }
+                                                ],
+                                                "name": "node_memory_Cached_bytes",
+                                                "offset": 0,
+                                                "type": "vectorSelector"
+                                            },
+                                            "type": "binaryExpr"
+                                        },
+                                        "matching": null,
+                                        "op": "+",
+                                        "rhs": {
+                                            "matchers": [
+                                                {
+                                                    "name": "job",
+                                                    "type": "=",
+                                                    "value": "node"
+                                                }
+                                            ],
+                                            "name": "node_memory_MemFree_bytes",
+                                            "offset": 0,
+                                            "type": "vectorSelector"
+                                        },
+                                        "type": "binaryExpr"
+                                    },
+                                    "matching": null,
+                                    "op": "+",
+                                    "rhs": {
+                                        "matchers": [
+                                            {
+                                                "name": "job",
+                                                "type": "=",
+                                                "value": "node"
+                                            }
+                                        ],
+                                        "name": "node_memory_Slab_bytes",
+                                        "offset": 0,
+                                        "type": "vectorSelector"
+                                    },
+                                    "type": "binaryExpr"
+                                },
+                                "type": "parenExpr"
+                            },
+                            "type": "binaryExpr"
+                        },
+                        "type": "parenExpr"
+                    },
+                    "matching": null,
+                    "op": "/",
+                    "rhs": {
+                        "matchers": [
+                            {
+                                "name": "job",
+                                "type": "=",
+                                "value": "node"
+                            }
+                        ],
+                        "name": "node_memory_MemTotal_bytes",
+                        "offset": 0,
+                        "type": "vectorSelector"
+                    },
+                    "type": "binaryExpr"
+                },
+                "type": "parenExpr"
+            },
+            "type": "binaryExpr"
+        }
+    );
 }

From 17e57700050e6471154e66b8506e083db9449bfd Mon Sep 17 00:00:00 2001
From: Ning Sun <sunning@greptime.com>
Date: Thu, 17 Oct 2024 16:00:59 -0700
Subject: [PATCH 6/8] test: cover more expression

---
 src/label/matcher.rs |   7 +--
 tests/ser.rs         | 104 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 6 deletions(-)

diff --git a/src/label/matcher.rs b/src/label/matcher.rs
index 9d05bae..5764d01 100644
--- a/src/label/matcher.rs
+++ b/src/label/matcher.rs
@@ -71,12 +71,7 @@ impl serde::Serialize for MatchOp {
     where
         S: serde::Serializer,
     {
-        match self {
-            MatchOp::Equal => serializer.serialize_str("="),
-            MatchOp::NotEqual => serializer.serialize_str("=~"),
-            MatchOp::Re(_reg) => serializer.serialize_str("=~"),
-            MatchOp::NotRe(_reg) => serializer.serialize_str("!~"),
-        }
+        serializer.serialize_str(&self.to_string())
     }
 }
 
diff --git a/tests/ser.rs b/tests/ser.rs
index 0ae3187..154f304 100644
--- a/tests/ser.rs
+++ b/tests/ser.rs
@@ -38,6 +38,55 @@ fn test_serialize() {
         "type": "vectorSelector"
     });
 
+    assert_json_ser_eq!(
+        "prometheus_tsdb_wal_writes_failed_total{label != \"nice\"}",
+
+        {
+            "matchers": [
+                {
+                    "name": "label",
+                    "type": "!=",
+                    "value": "nice"
+                }
+            ],
+            "name": "prometheus_tsdb_wal_writes_failed_total",
+            "offset": 0,
+            "type": "vectorSelector"
+        }
+    );
+
+    assert_json_ser_eq!(
+        "prometheus_tsdb_wal_writes_failed_total{label =~ \"nice\"}",
+
+    {
+        "matchers": [
+            {
+                "name": "label",
+                "type": "=~",
+                "value": "nice"
+            }
+        ],
+        "name": "prometheus_tsdb_wal_writes_failed_total",
+        "offset": 0,
+        "type": "vectorSelector"
+    });
+
+    assert_json_ser_eq!(
+        "prometheus_tsdb_wal_writes_failed_total{label !~ \"nice\"}",
+
+    {
+        "matchers": [
+            {
+                "name": "label",
+                "type": "!~",
+                "value": "nice"
+            }
+        ],
+        "name": "prometheus_tsdb_wal_writes_failed_total",
+        "offset": 0,
+        "type": "vectorSelector"
+    });
+
     assert_json_ser_eq!(
         "prometheus_tsdb_wal_writes_failed_total offset 2s @ start()",
 
@@ -278,4 +327,59 @@ fn test_serialize() {
             "type": "binaryExpr"
         }
     );
+
+    assert_json_ser_eq!("foo * on(branch) bar ",
+    {
+        "bool": false,
+        "lhs": {
+            "matchers": [],
+            "name": "foo",
+            "offset": 0,
+            "type": "vectorSelector"
+        },
+        "matching": {
+            "card": "one-to-one",
+            "include": [],
+            "labels": [
+            "branch"
+            ],
+            "on": true
+        },
+        "op": "*",
+        "rhs": {
+            "matchers": [],
+            "name": "bar",
+            "offset": 0,
+            "type": "vectorSelector"
+        },
+        "type": "binaryExpr"
+    });
+
+    assert_json_ser_eq!("foo * ignoring(branch) bar ",
+        {
+            "bool": false,
+            "lhs": {
+                "matchers": [],
+                "name": "foo",
+                "offset": 0,
+                "type": "vectorSelector"
+            },
+            "matching": {
+                "card": "one-to-one",
+                "include": [],
+                "labels": [
+                    "branch"
+                ],
+                "on": false
+            },
+            "op": "*",
+            "rhs": {
+                "matchers": [],
+                "name": "bar",
+                "offset": 0,
+                "type": "vectorSelector"
+            },
+            "type": "binaryExpr"
+        }
+    );
 }

From a85d78c57cb4d7ada7113fe0b814508b6465eb26 Mon Sep 17 00:00:00 2001
From: Ning Sun <sunning@greptime.com>
Date: Thu, 17 Oct 2024 16:14:30 -0700
Subject: [PATCH 7/8] fix: subquery

---
 src/parser/ast.rs    |  9 ++++++++-
 src/util/duration.rs | 12 ++++++++++++
 tests/ser.rs         | 43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/src/parser/ast.rs b/src/parser/ast.rs
index 8a57e95..6879ac9 100644
--- a/src/parser/ast.rs
+++ b/src/parser/ast.rs
@@ -655,8 +655,16 @@ pub struct SubqueryExpr {
     pub offset: Option<Offset>,
     #[cfg_attr(feature = "ser", serde(flatten))]
     pub at: Option<AtModifier>,
+    #[cfg_attr(
+        feature = "ser",
+        serde(serialize_with = "crate::util::duration::serialize_duration")
+    )]
     pub range: Duration,
     /// Default is the global evaluation interval.
+    #[cfg_attr(
+        feature = "ser",
+        serde(serialize_with = "crate::util::duration::serialize_duration_opt")
+    )]
     pub step: Option<Duration>,
 }
 
@@ -1036,7 +1044,6 @@ pub enum Expr {
     Paren(ParenExpr),
 
     /// SubqueryExpr represents a subquery.
-    #[cfg_attr(feature = "ser", serde(rename = "subqueryExpr"))]
     Subquery(SubqueryExpr),
 
     /// NumberLiteral represents a number.
diff --git a/src/util/duration.rs b/src/util/duration.rs
index 2621e1e..93a5c3c 100644
--- a/src/util/duration.rs
+++ b/src/util/duration.rs
@@ -144,6 +144,18 @@ where
     serializer.serialize_u128(duration_millis)
 }
 
+#[cfg(feature = "ser")]
+pub fn serialize_duration_opt<S>(dur: &Option<Duration>, serializer: S) -> Result<S::Ok, S::Error>
+where
+    S: serde::Serializer,
+{
+    if let Some(dur) = dur {
+        serialize_duration(dur, serializer)
+    } else {
+        serializer.serialize_none()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/tests/ser.rs b/tests/ser.rs
index 154f304..b7346e2 100644
--- a/tests/ser.rs
+++ b/tests/ser.rs
@@ -382,4 +382,47 @@ fn test_serialize() {
             "type": "binaryExpr"
         }
     );
+
+    assert_json_ser_eq!("min_over_time( rate(http_requests_total[5m])[30m:1m] )",
+        {
+            "args": [
+                {
+                    "expr": {
+                        "args": [
+                            {
+                                "matchers": [],
+                                "name": "http_requests_total",
+                                "offset": 0,
+                                "range": 300000,
+                                "type": "matrixSelector"
+                            }
+                        ],
+                        "func": {
+                            "argTypes": [
+                                "matrix"
+                            ],
+                            "name": "rate",
+                            "returnType": "vector",
+                            "variadic": 0
+                        },
+                        "type": "call"
+                    },
+                    "offset": 0,
+                    "range": 1800000,
+                    "step": 60000,
+                    "type": "subquery"
+                }
+            ],
+            "func": {
+                "argTypes": [
+                    "matrix"
+                ],
+                "name": "min_over_time",
+                "returnType": "vector",
+                "variadic": 0
+            },
+            "type": "call"
+
+    }
+        );
 }

From f7aed5539b38878c27bb6d24f5d40f97834eb5b9 Mon Sep 17 00:00:00 2001
From: Ning Sun <sunning@greptime.com>
Date: Mon, 21 Oct 2024 21:04:45 -0700
Subject: [PATCH 8/8] refactor: address review comments

---
 src/parser/ast.rs      | 10 +++++++---
 src/parser/function.rs |  2 +-
 src/util/duration.rs   |  7 +++++--
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/parser/ast.rs b/src/parser/ast.rs
index 6879ac9..141ad20 100644
--- a/src/parser/ast.rs
+++ b/src/parser/ast.rs
@@ -264,7 +264,8 @@ pub enum Offset {
 }
 
 impl Offset {
-    pub fn value_millis(&self) -> i128 {
+    #[cfg(feature = "ser")]
+    pub(crate) fn as_millis(&self) -> i128 {
         match self {
             Self::Pos(dur) => dur.as_millis() as i128,
             Self::Neg(dur) => -(dur.as_millis() as i128),
@@ -272,11 +273,14 @@ impl Offset {
     }
 
     #[cfg(feature = "ser")]
-    pub fn serialize_offset<S>(offset: &Option<Self>, serializer: S) -> Result<S::Ok, S::Error>
+    pub(crate) fn serialize_offset<S>(
+        offset: &Option<Self>,
+        serializer: S,
+    ) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
     {
-        let value = offset.as_ref().map(|o| o.value_millis()).unwrap_or(0);
+        let value = offset.as_ref().map(|o| o.as_millis()).unwrap_or(0);
         serializer.serialize_i128(value)
     }
 }
diff --git a/src/parser/function.rs b/src/parser/function.rs
index c05c42e..1b90fed 100644
--- a/src/parser/function.rs
+++ b/src/parser/function.rs
@@ -108,7 +108,7 @@ impl Function {
     }
 
     #[cfg(feature = "ser")]
-    pub fn serialize_variadic<S>(variadic: &bool, serializer: S) -> Result<S::Ok, S::Error>
+    pub(crate) fn serialize_variadic<S>(variadic: &bool, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
     {
diff --git a/src/util/duration.rs b/src/util/duration.rs
index 93a5c3c..472e3fd 100644
--- a/src/util/duration.rs
+++ b/src/util/duration.rs
@@ -136,7 +136,7 @@ pub fn display_duration(duration: &Duration) -> String {
 }
 
 #[cfg(feature = "ser")]
-pub fn serialize_duration<S>(dur: &Duration, serializer: S) -> Result<S::Ok, S::Error>
+pub(crate) fn serialize_duration<S>(dur: &Duration, serializer: S) -> Result<S::Ok, S::Error>
 where
     S: serde::Serializer,
 {
@@ -145,7 +145,10 @@ where
 }
 
 #[cfg(feature = "ser")]
-pub fn serialize_duration_opt<S>(dur: &Option<Duration>, serializer: S) -> Result<S::Ok, S::Error>
+pub(crate) fn serialize_duration_opt<S>(
+    dur: &Option<Duration>,
+    serializer: S,
+) -> Result<S::Ok, S::Error>
 where
     S: serde::Serializer,
 {