From dacdec7826e345359df625ce450dfd5056b1f466 Mon Sep 17 00:00:00 2001 From: Ning Sun Date: Mon, 25 May 2026 00:11:16 -0700 Subject: [PATCH 01/11] feat!: add function sync script with upstream (#138) * feat: add function sync script with upstream * fix: correct variadic -1 argument count in error message * chore: lint fix * chore: header * feat: sync functions, remove holt_winters, add start/end/range/step * chore: format * feat: add vardict test s * refactor: make arg count check easier to read * fix: when variadict is -1 it means the last argument is 0 or more --- .github/workflows/function-comparison.yml | 34 +++ scripts/compare_functions.py | 277 ++++++++++++++++++++++ src/parser/ast.rs | 187 ++++++++++++--- src/parser/function.rs | 82 ++++++- src/parser/parse.rs | 26 +- 5 files changed, 537 insertions(+), 69 deletions(-) create mode 100644 .github/workflows/function-comparison.yml create mode 100755 scripts/compare_functions.py diff --git a/.github/workflows/function-comparison.yml b/.github/workflows/function-comparison.yml new file mode 100644 index 0000000..405c563 --- /dev/null +++ b/.github/workflows/function-comparison.yml @@ -0,0 +1,34 @@ +# 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. + + +name: Function Comparison + +on: + schedule: + - cron: '0 0 * * 0' + workflow_dispatch: + +jobs: + compare: + name: Compare Functions + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.x' + - name: Run function comparison + run: python scripts/compare_functions.py diff --git a/scripts/compare_functions.py b/scripts/compare_functions.py new file mode 100755 index 0000000..4652320 --- /dev/null +++ b/scripts/compare_functions.py @@ -0,0 +1,277 @@ +#!/usr/bin/env python3 +""" +Script to compare Prometheus Go functions.go with Rust functions.rs +Ensures Rust functions are complete and consistent with Go version. +""" + +import re +import sys +from typing import Dict, List, Optional, Tuple +from dataclasses import dataclass + + +@dataclass +class FunctionDef: + name: str + arg_types: List[str] + variadic: int + return_type: str + experimental: bool + + def __str__(self): + exp_flag = " [EXP]" if self.experimental else "" + return f"{self.name}: args={self.arg_types}, variadic={self.variadic}, return={self.return_type}{exp_flag}" + + +def parse_go_functions(content: str) -> Dict[str, FunctionDef]: + """Parse Prometheus Go functions.go to extract function definitions.""" + functions = {} + + # Find all function entry points + entries = [] + pattern = r'"([^"]+)":\s*\{' + for match in re.finditer(pattern, content): + entries.append((match.start(), match.end(), match.group(1))) + + # Extract each function block + for i, (start, end, name) in enumerate(entries): + block_start = end + # Find the closing brace for this block + brace_count = 1 + block_end = block_start + while brace_count > 0 and block_end < len(content): + block_end += 1 + if content[block_end] == "{": + brace_count += 1 + elif content[block_end] == "}": + brace_count -= 1 + + block_content = content[block_start:block_end] + + # Parse Name field + name_match = re.search(r'Name:\s*"([^"]+)"', block_content) + if not name_match: + continue + + # Parse ArgTypes field + arg_types = [] + arg_types_match = re.search( + r"ArgTypes:\s*\[\]ValueType\{(.*?)\}", block_content, re.DOTALL + ) + if arg_types_match: + arg_types_str = arg_types_match.group(1).strip() + for arg in re.findall(r"ValueType(\w+)", arg_types_str): + arg_types.append(arg) + + # Parse Variadic field + variadic = 0 + variadic_match = re.search(r"Variadic:\s*(-?\d+)", block_content) + if variadic_match: + variadic = int(variadic_match.group(1)) + + # Parse ReturnType field + return_type = "" + return_match = re.search(r"ReturnType:\s*([^,\n}]+)", block_content) + if return_match: + return_type_clean = re.sub(r"ValueType", "", return_match.group(1)).strip() + return_type = return_type_clean + + # Parse Experimental field + experimental = False + experimental_match = re.search(r"Experimental:\s*(true|false)", block_content) + if experimental_match: + experimental = experimental_match.group(1).lower() == "true" + + functions[name] = FunctionDef( + name=name, + arg_types=arg_types, + variadic=variadic, + return_type=return_type, + experimental=experimental, + ) + + return functions + + +def parse_rust_functions(content: str) -> Dict[str, FunctionDef]: + """Parse Rust functions.rs to extract function definitions.""" + functions = {} + + # Pattern to match function! macro calls + # Example: + # function!("abs", vec![ValueType::Vector], 0, ValueType::Vector, false), + # function!("days_in_month", vec![ValueType::Vector], 1, ValueType::Vector, false), + # function!("label_join", vec![ValueType::Vector, ValueType::String, ValueType::String, ValueType::String], -1, ValueType::Vector, false), + # function!("double_exponential_smoothing", vec![ValueType::Matrix, ValueType::Scalar, ValueType::Scalar], 0, ValueType::Vector, true), + pattern = r'function!\(\s*"([^"]+)"\s*,\s*vec!\[(.*?)\]\s*,\s*(-?\d+)\s*,\s*ValueType::(\w+)\s*,\s*(true|false)\s*\),' + + for match in re.finditer(pattern, content, re.DOTALL): + name = match.group(1) + arg_types_str = match.group(2).strip() + variadic = int(match.group(3)) + return_type = match.group(4) + experimental = match.group(5).lower() == "true" + + # Parse arg types + arg_types = [] + if arg_types_str: + for arg in re.findall(r"ValueType::(\w+)", arg_types_str): + arg_types.append(arg) + + functions[name] = FunctionDef( + name=name, + arg_types=arg_types, + variadic=variadic, + return_type=return_type, + experimental=experimental, + ) + + return functions + + +def normalize_type(type_str: str) -> str: + """Normalize type names for comparison.""" + # Map Go types to Rust types + type_mapping = { + "String": "String", + "None": "None", + } + return type_mapping.get(type_str, type_str) + + +def compare_functions(go_func: FunctionDef, rust_func: FunctionDef) -> List[str]: + """Compare two function definitions and return list of differences.""" + differences = [] + + # Compare arg types + go_args = [normalize_type(t) for t in go_func.arg_types] + rust_args = [normalize_type(t) for t in rust_func.arg_types] + + if go_args != rust_args: + differences.append(f" Arg types differ: Go={go_args}, Rust={rust_args}") + + # Compare variadic + if go_func.variadic != rust_func.variadic: + differences.append( + f" Variadic differs: Go={go_func.variadic}, Rust={rust_func.variadic}" + ) + + # Compare return type + go_return = normalize_type(go_func.return_type) + rust_return = normalize_type(rust_func.return_type) + if go_return != rust_return: + differences.append(f" Return type differs: Go={go_return}, Rust={rust_return}") + + # Compare experimental flag + if go_func.experimental != rust_func.experimental: + differences.append( + f" Experimental flag differs: Go={go_func.experimental}, Rust={rust_func.experimental}" + ) + + return differences + + +def main(): + import subprocess + + # Fetch Prometheus Go functions.go from GitHub + go_url = "https://raw.githubusercontent.com/prometheus/prometheus/main/promql/parser/functions.go" + print(f"Fetching Prometheus functions.go from {go_url}...") + + try: + result = subprocess.run( + ["curl", "-s", go_url], capture_output=True, text=True, check=True + ) + go_content = result.stdout + except Exception as e: + print(f"Error fetching Go file: {e}") + sys.exit(1) + + # Read Rust functions.rs + rust_file = "src/parser/function.rs" + print(f"Reading Rust functions from {rust_file}...") + + try: + with open(rust_file, "r") as f: + rust_content = f.read() + except Exception as e: + print(f"Error reading Rust file: {e}") + sys.exit(1) + + # Parse both files + go_functions = parse_go_functions(go_content) + rust_functions = parse_rust_functions(rust_content) + + print(f"\nParsed {len(go_functions)} functions from Go") + print(f"Parsed {len(rust_functions)} functions from Rust\n") + + # Find missing functions in Rust + missing_in_rust = set(go_functions.keys()) - set(rust_functions.keys()) + + # Find extra functions in Rust + extra_in_rust = set(rust_functions.keys()) - set(go_functions.keys()) + + # Find differences in common functions + common_functions = set(go_functions.keys()) & set(rust_functions.keys()) + differences = {} + + for func_name in sorted(common_functions): + go_func = go_functions[func_name] + rust_func = rust_functions[func_name] + + diff = compare_functions(go_func, rust_func) + if diff: + differences[func_name] = (go_func, rust_func, diff) + + # Print results + print("=" * 80) + print("COMPARISON RESULTS") + print("=" * 80) + + if missing_in_rust: + print(f"\nāŒ {len(missing_in_rust)} function(s) MISSING in Rust:") + for func in sorted(missing_in_rust): + print(f" - {func}") + + if extra_in_rust: + print(f"\nāš ļø {len(extra_in_rust)} function(s) EXTRA in Rust (not in Go):") + for func in sorted(extra_in_rust): + print(f" - {func}") + + if differences: + print(f"\nšŸ” {len(differences)} function(s) have DIFFERENCES:") + for func_name in sorted(differences.keys()): + go_func, rust_func, diff = differences[func_name] + print(f"\n {func_name}:") + print(f" Go version: {go_func}") + print(f" Rust version: {rust_func}") + for d in diff: + print(f" {d}") + + # Summary + print("\n" + "=" * 80) + print("SUMMARY") + print("=" * 80) + + total_go = len(go_functions) + total_rust = len(rust_functions) + total_common = len(common_functions) + total_differences = len(differences) + + print(f"Go functions: {total_go}") + print(f"Rust functions: {total_rust}") + print(f"Common functions: {total_common}") + print(f"Missing in Rust: {len(missing_in_rust)}") + print(f"Extra in Rust: {len(extra_in_rust)}") + print(f"Differences: {total_differences}") + + if not missing_in_rust and not differences: + print("\nāœ… All functions are COMPLETE and CONSISTENT!") + sys.exit(0) + else: + print("\nāŒ Issues found - please review and fix") + sys.exit(1) + + +if __name__ == "__main__": + main() diff --git a/src/parser/ast.rs b/src/parser/ast.rs index f79e9a2..52bddb5 100644 --- a/src/parser/ast.rs +++ b/src/parser/ast.rs @@ -1618,34 +1618,14 @@ fn check_ast_for_aggregate_expr(ex: AggregateExpr) -> Result { } fn check_ast_for_call(ex: Call) -> Result { - let expected_args_len = ex.func.arg_types.len(); let name = ex.func.name; - let actual_args_len = ex.args.len(); - if ex.func.variadic == 0 { - if expected_args_len != actual_args_len { - return Err(format!( - "expected {expected_args_len} argument(s) in call to '{name}', got {actual_args_len}" - )); - } - } else { - let expected_args_len_without_default = expected_args_len.saturating_sub(1); - if expected_args_len_without_default > actual_args_len { - return Err(format!( - "expected at least {expected_args_len_without_default} argument(s) in call to '{name}', got {actual_args_len}" - )); - } - - if ex.func.variadic > 0 { - let expected_max_args_len = - expected_args_len_without_default + ex.func.variadic as usize; - if expected_max_args_len < actual_args_len { - return Err(format!( - "expected at most {expected_max_args_len} argument(s) in call to '{name}', got {actual_args_len}" - )); - } - } - } + check_call_arity( + ex.func.arg_types.len(), + ex.func.variadic, + ex.args.len(), + name, + )?; // special cases from https://prometheus.io/docs/prometheus/latest/querying/functions if name.eq("exp") { @@ -1662,20 +1642,53 @@ fn check_ast_for_call(ex: Call) -> Result { } } - for (mut idx, actual_arg) in ex.args.args.iter().enumerate() { - // this only happens when function args are variadic - if idx >= ex.func.arg_types.len() { - idx = ex.func.arg_types.len() - 1; + check_args_match_types(&ex.args.args, &ex.func.arg_types, name)?; + Ok(Expr::Call(ex)) +} + +fn check_call_arity(nargs: usize, variadic: i32, actual: usize, name: &str) -> Result<(), String> { + if variadic == 0 { + if nargs != actual { + return Err(format!( + "expected {nargs} argument(s) in call to '{name}', got {actual}" + )); + } + } else { + let na = nargs.saturating_sub(1); + if na > actual { + return Err(format!( + "expected at least {na} argument(s) in call to '{name}', got {actual}" + )); + } else if variadic > 0 { + let nargsmax = na + variadic as usize; + if nargsmax < actual { + return Err(format!( + "expected at most {nargsmax} argument(s) in call to '{name}', got {actual}" + )); + } } + } + Ok(()) +} +fn check_args_match_types( + args: &[Box], + arg_types: &[ValueType], + name: &str, +) -> Result<(), String> { + for (i, actual_arg) in args.iter().enumerate() { + let expected_idx = if i < arg_types.len() { + i + } else { + arg_types.len() - 1 + }; expect_type( - ex.func.arg_types[idx], + arg_types[expected_idx], Some(actual_arg.value_type()), &format!("call to function '{name}'"), )?; } - - Ok(Expr::Call(ex)) + Ok(()) } fn check_ast_for_unary(ex: UnaryExpr) -> Result { @@ -2812,6 +2825,114 @@ or assert_eq!(expect, stmt.to_string()); } + fn make_call(func_name: &str, arg_count: usize) -> Call { + use crate::parser::function::get_function; + let func = + get_function(func_name).unwrap_or_else(|| panic!("unknown function: {func_name}")); + let args: Vec> = (0..arg_count) + .map(|_| Box::new(Expr::VectorSelector(VectorSelector::from("foo")))) + .collect(); + Call { + func, + args: FunctionArgs { args }, + } + } + + #[test] + fn test_call_arity_variadic_zero() { + // floor: arg_types=[Vector], variadic=0 → exact 1 arg required + assert!(check_ast(Expr::Call(make_call("floor", 1))).is_ok()); + + let err = check_ast(Expr::Call(make_call("floor", 0))).unwrap_err(); + assert!( + err.contains("expected 1 argument(s) in call to 'floor', got 0"), + "{err}" + ); + + let err = check_ast(Expr::Call(make_call("floor", 2))).unwrap_err(); + assert!( + err.contains("expected 1 argument(s) in call to 'floor', got 2"), + "{err}" + ); + } + + #[test] + fn test_call_arity_bounded_variadic_single_arg_type() { + // days_in_month: arg_types=[Vector], variadic=1 → min=0, max=1 + // 0 args is valid (default); only "too many" is enforced + assert!(check_ast(Expr::Call(make_call("days_in_month", 1))).is_ok()); + + let err = check_ast(Expr::Call(make_call("days_in_month", 2))).unwrap_err(); + assert!( + err.contains("expected at most 1 argument(s) in call to 'days_in_month', got 2"), + "{err}" + ); + } + + #[test] + fn test_call_arity_bounded_variadic_two_arg_types() { + // round: arg_types=[Vector, Scalar], variadic=1 → min=1, max=2 + let err = check_ast(Expr::Call(make_call("round", 0))).unwrap_err(); + assert!( + err.contains("expected at least 1 argument(s) in call to 'round', got 0"), + "{err}" + ); + + let err = check_ast(Expr::Call(make_call("round", 3))).unwrap_err(); + assert!( + err.contains("expected at most 2 argument(s) in call to 'round', got 3"), + "{err}" + ); + + // info: arg_types=[Vector, Vector], variadic=1 → min=1, max=2 + let err = check_ast(Expr::Call(make_call("info", 0))).unwrap_err(); + assert!( + err.contains("expected at least 1 argument(s) in call to 'info', got 0"), + "{err}" + ); + + let err = check_ast(Expr::Call(make_call("info", 3))).unwrap_err(); + assert!( + err.contains("expected at most 2 argument(s) in call to 'info', got 3"), + "{err}" + ); + } + + #[test] + fn test_call_arity_bounded_variadic_large() { + // histogram_quantiles: arg_types=[Vector, String, Scalar, Scalar], variadic=9 → min=3, max=12 + let err = check_ast(Expr::Call(make_call("histogram_quantiles", 2))).unwrap_err(); + assert!( + err.contains("expected at least 3 argument(s) in call to 'histogram_quantiles', got 2"), + "{err}" + ); + + let err = check_ast(Expr::Call(make_call("histogram_quantiles", 13))).unwrap_err(); + assert!( + err.contains( + "expected at most 12 argument(s) in call to 'histogram_quantiles', got 13" + ), + "{err}" + ); + } + + #[test] + fn test_call_arity_unbounded_variadic() { + // label_join: arg_types=[Vector, String, String, String], variadic=-1 → min=3, no max + let err = check_ast(Expr::Call(make_call("label_join", 2))).unwrap_err(); + assert!( + err.contains("expected at least 3 argument(s) in call to 'label_join', got 2"), + "{err}" + ); + + // sort_by_label: arg_types=[Vector, String], variadic=-1 → min=1, no max + let err = check_ast(Expr::Call(make_call("sort_by_label", 0))).unwrap_err(); + assert!( + err.contains("expected at least 1 argument(s) in call to 'sort_by_label', got 0"), + "{err}" + ); + } + #[test] fn test_prettify_with_utf8_labels() { // Test that labels with special characters are properly quoted in display diff --git a/src/parser/function.rs b/src/parser/function.rs index 8afcc0c..158c5d7 100644 --- a/src/parser/function.rs +++ b/src/parser/function.rs @@ -247,30 +247,31 @@ lazy_static! { ValueType::Vector, false ), + function!("end", vec![], 0, ValueType::Scalar, true), function!("exp", vec![ValueType::Vector], 0, ValueType::Vector, false), function!( - "floor", - vec![ValueType::Vector], + "first_over_time", + vec![ValueType::Matrix], 0, ValueType::Vector, - false + true ), function!( - "histogram_count", + "floor", vec![ValueType::Vector], 0, ValueType::Vector, false ), function!( - "histogram_sum", + "histogram_avg", vec![ValueType::Vector], 0, ValueType::Vector, false ), function!( - "histogram_avg", + "histogram_count", vec![ValueType::Vector], 0, ValueType::Vector, @@ -290,6 +291,18 @@ lazy_static! { ValueType::Vector, false ), + function!( + "histogram_quantiles", + vec![ + ValueType::Vector, + ValueType::String, + ValueType::Scalar, + ValueType::Scalar + ], + 9, + ValueType::Vector, + true + ), function!( "histogram_stddev", vec![ValueType::Vector], @@ -305,18 +318,25 @@ lazy_static! { false ), function!( - "double_exponential_smoothing", - vec![ValueType::Matrix, ValueType::Scalar, ValueType::Scalar], + "histogram_sum", + vec![ValueType::Vector], 0, ValueType::Vector, + false + ), + function!( + "info", + vec![ValueType::Vector, ValueType::Vector], + 1, + ValueType::Vector, true ), function!( - "holt_winters", + "double_exponential_smoothing", vec![ValueType::Matrix, ValueType::Scalar, ValueType::Scalar], 0, ValueType::Vector, - false + true ), function!("hour", vec![ValueType::Vector], 1, ValueType::Vector, false), function!( @@ -381,6 +401,13 @@ lazy_static! { false ), function!("log2", vec![ValueType::Vector], 0, ValueType::Vector, false), + function!( + "mad_over_time", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true + ), function!( "max_over_time", vec![ValueType::Matrix], @@ -395,6 +422,34 @@ lazy_static! { ValueType::Vector, false ), + function!( + "ts_of_first_over_time", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true + ), + function!( + "ts_of_last_over_time", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true + ), + function!( + "ts_of_max_over_time", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true + ), + function!( + "ts_of_min_over_time", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true + ), function!( "minute", vec![ValueType::Vector], @@ -410,6 +465,7 @@ lazy_static! { false ), function!("pi", vec![], 0, ValueType::Scalar, false), + function!("range", vec![], 0, ValueType::Scalar, true), function!( "predict_linear", vec![ValueType::Matrix, ValueType::Scalar], @@ -455,6 +511,8 @@ lazy_static! { false ), function!("sgn", vec![ValueType::Vector], 0, ValueType::Vector, false), + function!("start", vec![], 0, ValueType::Scalar, true), + function!("step", vec![], 0, ValueType::Scalar, true), function!("sin", vec![ValueType::Vector], 0, ValueType::Vector, false), function!("sinh", vec![ValueType::Vector], 0, ValueType::Vector, false), function!("sort", vec![ValueType::Vector], 0, ValueType::Vector, false), @@ -467,14 +525,14 @@ lazy_static! { ), function!( "sort_by_label", - vec![ValueType::Vector, ValueType::String, ValueType::String], + vec![ValueType::Vector, ValueType::String], -1, ValueType::Vector, true ), function!( "sort_by_label_desc", - vec![ValueType::Vector, ValueType::String, ValueType::String], + vec![ValueType::Vector, ValueType::String], -1, ValueType::Vector, true diff --git a/src/parser/parse.rs b/src/parser/parse.rs index 97bedd2..c4b4e49 100644 --- a/src/parser/parse.rs +++ b/src/parser/parse.rs @@ -1370,20 +1370,6 @@ mod tests { ) }) }), - ("holt_winters(some_metric[5m], 0.5, 0.1)", { - Expr::new_matrix_selector( - Expr::from(VectorSelector::from("some_metric")), - duration::MINUTE_DURATION * 5, - ) - .and_then(|ex| { - Expr::new_call( - get_function("holt_winters").unwrap(), - FunctionArgs::new_args(ex) - .append_args(Expr::from(0.5)) - .append_args(Expr::from(0.1)), - ) - }) - }), // cases from https://prometheus.io/docs/prometheus/latest/querying/functions (r#"absent(nonexistent{job="myjob"})"#, { let name = String::from("nonexistent"); @@ -1880,19 +1866,11 @@ mod tests { ), ( "sort_by_label()", - "expected at least 2 argument(s) in call to 'sort_by_label', got 0", + "expected at least 1 argument(s) in call to 'sort_by_label', got 0", ), ( "sort_by_label_desc()", - "expected at least 2 argument(s) in call to 'sort_by_label_desc', got 0", - ), - ( - "sort_by_label(sum(up) by (instance))", - "expected at least 2 argument(s) in call to 'sort_by_label', got 1", - ), - ( - "sort_by_label_desc(sum(up) by (instance))", - "expected at least 2 argument(s) in call to 'sort_by_label_desc', got 1", + "expected at least 1 argument(s) in call to 'sort_by_label_desc', got 0", ), // (r#"label_replace(a, `b`, `c\xff`, `d`, `.*`)"#, ""), ]; From 473a97607c4e95471f7a0310f598796f6ca98149 Mon Sep 17 00:00:00 2001 From: Harry John Date: Wed, 27 May 2026 21:06:07 -0700 Subject: [PATCH 02/11] fix: re-escape backslashes and quotes in Display/prettify output (#146) --- src/label/matcher.rs | 6 +++--- src/label/mod.rs | 3 ++- src/parser/ast.rs | 42 +++++++++++++++++++++++++++++++++++++++-- src/util/mod.rs | 2 +- src/util/string.rs | 45 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 7 deletions(-) diff --git a/src/label/matcher.rs b/src/label/matcher.rs index 6823248..3174a53 100644 --- a/src/label/matcher.rs +++ b/src/label/matcher.rs @@ -19,7 +19,7 @@ use regex::Regex; use crate::parser::lex::is_label; use crate::parser::token::{token_display, TokenId, T_EQL, T_EQL_REGEX, T_NEQ, T_NEQ_REGEX}; -use crate::util::join_vector; +use crate::util::{escape_string, join_vector}; const LABEL_METRIC_NAME: &str = "__name__"; @@ -152,9 +152,9 @@ impl fmt::Display for Matcher { let name = if is_label(&self.name) { self.name.clone() } else { - format!("\"{}\"", self.name) + format!("\"{}\"", escape_string(&self.name)) }; - write!(f, "{}{}\"{}\"", name, self.op, self.value) + write!(f, "{}{}\"{}\"", name, self.op, escape_string(&self.value)) } } diff --git a/src/label/mod.rs b/src/label/mod.rs index c05ff3e..4e1ac11 100644 --- a/src/label/mod.rs +++ b/src/label/mod.rs @@ -18,6 +18,7 @@ use std::collections::HashSet; use std::fmt; use crate::parser::lex::is_label; +use crate::util::escape_string; mod matcher; pub use matcher::{MatchOp, Matcher, Matchers}; @@ -78,7 +79,7 @@ impl fmt::Display for Labels { if is_label(label) { label.clone() } else { - format!("\"{}\"", label) + format!("\"{}\"", escape_string(label)) } }) .collect(); diff --git a/src/parser/ast.rs b/src/parser/ast.rs index 52bddb5..5c3b1f7 100644 --- a/src/parser/ast.rs +++ b/src/parser/ast.rs @@ -19,7 +19,7 @@ use crate::parser::token::{ use crate::parser::token::{Token, TokenId, TokenType}; use crate::parser::value::ValueType; use crate::parser::{indent, Function, FunctionArgs, Prettier, MAX_CHARACTERS_PER_LINE}; -use crate::util::display_duration; +use crate::util::{display_duration, escape_string}; use chrono::{DateTime, Utc}; use std::fmt::{self, Write}; use std::ops::Neg; @@ -870,7 +870,7 @@ pub struct StringLiteral { impl fmt::Display for StringLiteral { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "\"{}\"", self.val) + write!(f, "\"{}\"", escape_string(&self.val)) } } @@ -2973,4 +2973,42 @@ or assert_eq!(prettified, expected); } } + + #[test] + fn test_prettify_escape_roundtrip() { + // Queries with backslash escapes must survive parse → prettify → re-parse + let cases = vec![ + // Escaped dot in regex matcher + ( + r#"{__name__="up",service=~"flagd\\.evaluation\\.v1\\.Service"}"#, + r#"{__name__="up",service=~"flagd\\.evaluation\\.v1\\.Service"}"#, + ), + // Escaped pipe + ( + r#"{__name__="up",tag=~"a\\|b"}"#, + r#"{__name__="up",tag=~"a\\|b"}"#, + ), + // Literal backslash in value + (r#"{path="C:\\\\Windows"}"#, r#"{path="C:\\\\Windows"}"#), + // Embedded double quote + (r#"{msg="say \"hello\""}"#, r#"{msg="say \"hello\""}"#), + ]; + + for (input, expected) in &cases { + let parsed = crate::parser::parse(input).unwrap(); + let prettified = parsed.prettify(); + assert_eq!( + &prettified, expected, + "prettify mismatch for input: {input}" + ); + + // Roundtrip: re-parsing the prettified output must succeed and produce the same result + let reparsed = crate::parser::parse(&prettified).unwrap(); + assert_eq!( + parsed.prettify(), + reparsed.prettify(), + "roundtrip failed for input: {input}" + ); + } + } } diff --git a/src/util/mod.rs b/src/util/mod.rs index e59c4a1..46e3209 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -21,7 +21,7 @@ pub mod visitor; pub use duration::{display_duration, parse_duration}; pub use number::parse_str_radix; -pub use string::unquote_string; +pub use string::{escape_string, unquote_string}; pub use visitor::{walk_expr, ExprVisitor}; pub(crate) fn join_vector(v: &[T], sep: &str, sort: bool) -> String { diff --git a/src/util/string.rs b/src/util/string.rs index aa389c8..b8d96d0 100644 --- a/src/util/string.rs +++ b/src/util/string.rs @@ -14,6 +14,23 @@ //! Internal utilities for strings. +/// Escapes a string value for embedding in a PromQL double-quoted string literal. +/// This is the inverse of `unquote_string` — it re-escapes backslashes and double quotes. +pub fn escape_string(s: &str) -> String { + let mut result = String::with_capacity(s.len()); + for c in s.chars() { + match c { + '\\' => result.push_str("\\\\"), + '"' => result.push_str("\\\""), + '\n' => result.push_str("\\n"), + '\r' => result.push_str("\\r"), + '\t' => result.push_str("\\t"), + _ => result.push(c), + } + } + result +} + /// This function is modified from original go version /// https://github.com/prometheus/prometheus/blob/v3.8.0/util/strutil/quote.go pub fn unquote_string(s: &str) -> Result { @@ -346,4 +363,32 @@ mod tests { assert!(unquote_string("`hello`world`").is_err()); assert!(unquote_string("``hello`").is_err()); } + + #[test] + fn test_escape_string() { + assert_eq!(escape_string("hello"), "hello"); + assert_eq!(escape_string(r#"say "hi""#), r#"say \"hi\""#); + assert_eq!(escape_string("back\\slash"), "back\\\\slash"); + assert_eq!(escape_string("new\nline"), "new\\nline"); + assert_eq!(escape_string("tab\there"), "tab\\there"); + assert_eq!(escape_string("cr\rhere"), "cr\\rhere"); + } + + #[test] + fn test_escape_unquote_roundtrip() { + // escape_string should produce output that unquote_string can reverse + let values = vec![ + "hello", + "flagd\\.eval", + "a\\|b", + "C:\\\\Windows", + "say \"hi\"", + ]; + for val in values { + let escaped = escape_string(val); + let quoted = format!("\"{}\"", escaped); + let unquoted = unquote_string("ed).unwrap(); + assert_eq!(unquoted, val, "roundtrip failed for: {val:?}"); + } + } } From ed72d2d70315fa698021de4f378c79147e0c759c Mon Sep 17 00:00:00 2001 From: Yingwen Date: Mon, 8 Jun 2026 16:19:58 +0800 Subject: [PATCH 03/11] feat: Add Prometheus min_of and max_of functions (#148) Add Prometheus min_of and max_of functions --- src/parser/function.rs | 22 ++++++++++++++++++++++ src/parser/parse.rs | 12 ++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/parser/function.rs b/src/parser/function.rs index 158c5d7..094fbf7 100644 --- a/src/parser/function.rs +++ b/src/parser/function.rs @@ -385,6 +385,13 @@ lazy_static! { ValueType::Vector, false ), + function!( + "max_of", + vec![ValueType::Scalar, ValueType::Scalar], + 0, + ValueType::Scalar, + true + ), function!( "last_over_time", vec![ValueType::Matrix], @@ -392,6 +399,13 @@ lazy_static! { ValueType::Vector, false ), + function!( + "min_of", + vec![ValueType::Scalar, ValueType::Scalar], + 0, + ValueType::Scalar, + true + ), function!("ln", vec![ValueType::Vector], 0, ValueType::Vector, false), function!( "log10", @@ -654,5 +668,13 @@ mod tests { let rate = get_function("rate").unwrap(); assert_eq!(rate.variadic, 0); assert!(!rate.experimental); + + for func_name in ["max_of", "min_of"] { + let func = get_function(func_name).unwrap(); + assert_eq!(func.arg_types, vec![ValueType::Scalar, ValueType::Scalar]); + assert_eq!(func.variadic, 0); + assert_eq!(func.return_type, ValueType::Scalar); + assert!(func.experimental); + } } } diff --git a/src/parser/parse.rs b/src/parser/parse.rs index c4b4e49..3d706d5 100644 --- a/src/parser/parse.rs +++ b/src/parser/parse.rs @@ -1356,6 +1356,18 @@ mod tests { FunctionArgs::new_args(ex).append_args(Expr::from(5.0)), ) }), + ("max_of(1, 2)", { + Expr::new_call( + get_function("max_of").unwrap(), + FunctionArgs::new_args(Expr::from(1.0)).append_args(Expr::from(2.0)), + ) + }), + ("min_of(1, 2)", { + Expr::new_call( + get_function("min_of").unwrap(), + FunctionArgs::new_args(Expr::from(1.0)).append_args(Expr::from(2.0)), + ) + }), ("double_exponential_smoothing(some_metric[5m], 0.5, 0.1)", { Expr::new_matrix_selector( Expr::from(VectorSelector::from("some_metric")), From 547724a778703306c35674c0d595550890f7a771 Mon Sep 17 00:00:00 2001 From: Yingwen Date: Mon, 8 Jun 2026 17:30:00 +0800 Subject: [PATCH 04/11] chore: Bump promql-parser crate version to 0.10.0 (#149) chore: bump crate version to 0.10.0 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 08743c4..b3e0a3e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ name = "promql-parser" readme = "README.md" description = "Parse PromQL query into AST" repository = "https://github.com/GreptimeTeam/promql-parser" -version = "0.9.0" +version = "0.10.0" edition = "2021" authors = ["The GreptimeDB Project Developers"] keywords = ["prometheus", "promql", "parser"] From d11a644172232298fa146d9c7ac578c2477bc053 Mon Sep 17 00:00:00 2001 From: Ben Ye Date: Mon, 17 Aug 2026 00:20:00 -0700 Subject: [PATCH 05/11] feat(function): Add custom function registration API (#150) * feat(function): Add custom function registration API Add register_extra_functions() and clear_extra_functions() to allow users to extend the parser with custom functions not in upstream Prometheus. This is inspired by Thanos's XFunctions pattern (thanos-io/promql-engine/execution/parse/functions.go) where custom functions like xrate/xdelta/xincrease are registered alongside built-in Prometheus functions. The implementation uses a RwLock-protected HashMap that get_function() falls back to when a function name is not found in the built-in FUNCTIONS map. This allows: - Registering functions once at startup (zero read-path contention) - Calling register_extra_functions() multiple times (additive) - Full argument type validation at parse time (same as built-ins) - Rejecting registrations that conflict with built-in function names Closes GreptimeTeam#144 * Apply suggestion from @sunng87 * style: Apply rustfmt formatting --------- Co-authored-by: Ning Sun --- src/parser/function.rs | 125 +++++++++++++++++++++++++++++++++++++- src/parser/mod.rs | 2 +- tests/custom_functions.rs | 91 +++++++++++++++++++++++++++ 3 files changed, 216 insertions(+), 2 deletions(-) create mode 100644 tests/custom_functions.rs diff --git a/src/parser/function.rs b/src/parser/function.rs index 094fbf7..5ca2baf 100644 --- a/src/parser/function.rs +++ b/src/parser/function.rs @@ -14,6 +14,7 @@ use std::collections::HashMap; use std::fmt; +use std::sync::RwLock; use lazy_static::lazy_static; @@ -21,6 +22,54 @@ use crate::parser::value::ValueType; use crate::parser::{Expr, Prettier}; use crate::util::join_vector; +lazy_static! { + static ref EXTRA_FUNCTIONS: RwLock> = RwLock::new(HashMap::new()); +} + +/// Register additional custom functions that the parser will recognize. +/// +/// This allows extending the parser with functions not in upstream Prometheus, +/// similar to Thanos's XFunctions pattern. Can be called multiple times; +/// functions are accumulated additively. +/// +/// Returns an error if any function name conflicts with a built-in Prometheus +/// function. Overriding built-in functions is not allowed. +/// +/// # Example +/// +/// ``` +/// use promql_parser::parser::function::{register_extra_functions, Function}; +/// use promql_parser::parser::value::ValueType; +/// +/// register_extra_functions(vec![ +/// Function::new("xrate", vec![ValueType::Matrix], 0, ValueType::Vector, true), +/// ]).unwrap(); +/// +/// // Now parse() will accept xrate as a valid function +/// let result = promql_parser::parser::parse("xrate(foo[5m])"); +/// assert!(result.is_ok()); +/// ``` +pub fn register_extra_functions(funcs: Vec) -> Result<(), String> { + let mut map = EXTRA_FUNCTIONS.write().unwrap(); + for f in &funcs { + if FUNCTIONS.contains_key(f.name) { + return Err(format!( + "cannot register custom function \"{}\": conflicts with built-in function", + f.name + )); + } + } + for f in funcs { + map.insert(f.name.to_string(), f); + } + Ok(()) +} + +/// Clear all previously registered custom functions. +pub fn clear_extra_functions() { + EXTRA_FUNCTIONS.write().unwrap().clear(); +} + /// called by func in Call #[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "ser", derive(serde::Serialize))] @@ -595,8 +644,12 @@ lazy_static! { } /// get_function returns a predefined Function object for the given name. +/// It checks built-in functions first, then any registered custom functions. pub(crate) fn get_function(name: &str) -> Option { - FUNCTIONS.get(name).cloned() + FUNCTIONS + .get(name) + .cloned() + .or_else(|| EXTRA_FUNCTIONS.read().unwrap().get(name).cloned()) } #[cfg(test)] @@ -677,4 +730,74 @@ mod tests { assert!(func.experimental); } } + + #[test] + fn test_register_custom_function() { + // Register a custom function + register_extra_functions(vec![Function::new( + "xrate", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true, + )]) + .unwrap(); + + // Should be found by get_function + let func = get_function("xrate"); + assert!(func.is_some()); + let func = func.unwrap(); + assert_eq!(func.name, "xrate"); + assert_eq!(func.arg_types, vec![ValueType::Matrix]); + assert_eq!(func.variadic, 0); + assert_eq!(func.return_type, ValueType::Vector); + assert!(func.experimental); + + // Built-in functions still work + assert!(get_function("rate").is_some()); + assert!(get_function("sum_over_time").is_some()); + + // Unknown functions still return None + assert!(get_function("totally_nonexistent_xyz").is_none()); + } + + #[test] + fn test_register_multiple_custom_functions() { + register_extra_functions(vec![ + Function::new( + "xdelta", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true, + ), + Function::new( + "xincrease", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true, + ), + ]) + .unwrap(); + + assert!(get_function("xdelta").is_some()); + assert!(get_function("xincrease").is_some()); + } + + #[test] + fn test_register_builtin_override_rejected() { + // Attempting to override a built-in function should return an error + let result = register_extra_functions(vec![Function::new( + "rate", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true, + )]); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .contains("conflicts with built-in function")); + } } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index be98c33..c89efbf 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -33,7 +33,7 @@ pub use ast::{ LabelModifier, MatrixSelector, NumberLiteral, Offset, ParenExpr, StringLiteral, SubqueryExpr, UnaryExpr, VectorMatchCardinality, VectorMatchFillValues, VectorSelector, }; -pub use function::{Function, FunctionArgs}; +pub use function::{clear_extra_functions, register_extra_functions, Function, FunctionArgs}; pub use lex::lexer; pub use parse::parse; diff --git a/tests/custom_functions.rs b/tests/custom_functions.rs new file mode 100644 index 0000000..72c042c --- /dev/null +++ b/tests/custom_functions.rs @@ -0,0 +1,91 @@ +// 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. + +use promql_parser::parser; +use promql_parser::parser::function::Function; +use promql_parser::parser::value::ValueType; + +#[test] +fn test_parse_with_custom_function() { + // Register custom functions (like Thanos xrate/xdelta/xincrease) + parser::register_extra_functions(vec![ + Function::new("xrate", vec![ValueType::Matrix], 0, ValueType::Vector, true), + Function::new( + "xdelta", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true, + ), + Function::new( + "xincrease", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true, + ), + ]) + .unwrap(); + + // Custom functions should parse successfully + let result = parser::parse("xrate(http_requests_total[5m])"); + assert!(result.is_ok(), "xrate should parse: {:?}", result.err()); + + let result = parser::parse("xdelta(temperature[1h])"); + assert!(result.is_ok(), "xdelta should parse: {:?}", result.err()); + + let result = parser::parse("xincrease(counter[10m])"); + assert!(result.is_ok(), "xincrease should parse: {:?}", result.err()); + + // Custom functions work in nested expressions + let result = parser::parse("sum(xrate(http_requests_total[5m])) by (instance)"); + assert!( + result.is_ok(), + "nested xrate should parse: {:?}", + result.err() + ); + + // Built-in functions still work + let result = parser::parse("rate(http_requests_total[5m])"); + assert!(result.is_ok(), "built-in rate should still work"); + + // Unknown functions still fail + let result = parser::parse("totally_unknown_func(up)"); + assert!(result.is_err(), "unknown function should still fail"); +} + +#[test] +fn test_custom_function_arg_validation() { + // Register a function that expects a matrix arg + parser::register_extra_functions(vec![Function::new( + "custom_over_time", + vec![ValueType::Matrix], + 0, + ValueType::Vector, + true, + )]) + .unwrap(); + + // Correct usage (matrix selector) + let result = parser::parse("custom_over_time(metric[5m])"); + assert!( + result.is_ok(), + "correct args should parse: {:?}", + result.err() + ); + + // Wrong arg type (vector instead of matrix) should fail + let result = parser::parse("custom_over_time(metric)"); + assert!(result.is_err(), "vector arg for matrix param should fail"); +} From b962552899c98c3b08c2690026da08891489aa53 Mon Sep 17 00:00:00 2001 From: Maxim Sokolov Date: Sun, 23 Aug 2026 05:03:48 -0700 Subject: [PATCH 06/11] build: bump grmtools to 0.14.2 to stop re-decoding parser tables per parse (#151) lrpar 0.13 generates a `parse()` that calls `_reconstitute` on every invocation, deserializing the grammar and LR state table (~47 KiB of tables, ~57 KiB of allocation per call). lrpar 0.14.2 caches them in a `OnceLock` (softdevteam/grmtools#631). On a 14-core M-series, `parse("sum(up)")` drops from ~25 us to ~7 us and steady-state allocation from ~61 KB to ~4.6 KB per parse; 16 threads parsing concurrently go from 438k to 1.48M parses/s. Also: - switch the deprecated `ct_token_map` to `CTTokenMapBuilder` - enable serde's `derive` feature explicitly: the `ser` feature relied on grmtools 0.13 enabling it transitively, which 0.14 no longer does Claude-Session: https://claude.ai/code/session_01VrH837ZJiH2gpReQPza32S Co-authored-by: Claude Fable 5 --- Cargo.toml | 14 +++++++------- build.rs | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b3e0a3e..e44ed39 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,13 +25,13 @@ license = "Apache-2.0" categories = ["parser-implementations"] [dependencies] -cfgrammar = "0.13.5" +cfgrammar = "0.14.2" chrono = "0.4.38" lazy_static = "1.4.0" -lrlex = "0.13.5" -lrpar = "0.13.5" +lrlex = "0.14.2" +lrpar = "0.14.2" regex = "1" -serde = { version = "1", optional = true } +serde = { version = "1", features = ["derive"], optional = true } serde_json = { version = "1", optional = true } [features] @@ -39,6 +39,6 @@ default = [] ser = ["serde", "serde_json"] [build-dependencies] -cfgrammar = "0.13.5" -lrlex = "0.13.5" -lrpar = "0.13.5" +cfgrammar = "0.14.2" +lrlex = "0.14.2" +lrpar = "0.14.2" diff --git a/build.rs b/build.rs index 8e3442c..8118be8 100644 --- a/build.rs +++ b/build.rs @@ -13,7 +13,7 @@ // limitations under the License. use cfgrammar::yacc::YaccKind; -use lrlex::{ct_token_map, DefaultLexerTypes}; +use lrlex::{CTTokenMapBuilder, DefaultLexerTypes}; use lrpar::CTParserBuilder; fn main() -> Result<(), Box> { @@ -22,5 +22,5 @@ fn main() -> Result<(), Box> { .recoverer(lrpar::RecoveryKind::None) .grammar_in_src_dir("parser/promql.y")? .build()?; - ct_token_map::("token_map", ctp.token_map(), None) + CTTokenMapBuilder::::new("token_map", ctp.token_map()).build() } From 2e4d6ec2cda535c57a5d6e9b574455c2fb8d5026 Mon Sep 17 00:00:00 2001 From: Ning Sun Date: Sun, 20 Sep 2026 18:50:03 +0800 Subject: [PATCH 07/11] fix: start/end as function (#153) --- src/parser/function.rs | 9 ++++++++- src/parser/parse.rs | 15 +++++++++------ src/parser/promql.y | 8 ++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/parser/function.rs b/src/parser/function.rs index 5ca2baf..4d9a717 100644 --- a/src/parser/function.rs +++ b/src/parser/function.rs @@ -303,7 +303,7 @@ lazy_static! { vec![ValueType::Matrix], 0, ValueType::Vector, - true + false ), function!( "floor", @@ -575,6 +575,13 @@ lazy_static! { ), function!("sgn", vec![ValueType::Vector], 0, ValueType::Vector, false), function!("start", vec![], 0, ValueType::Scalar, true), + function!( + "start_timestamp", + vec![ValueType::Vector], + 0, + ValueType::Vector, + true + ), function!("step", vec![], 0, ValueType::Scalar, true), function!("sin", vec![ValueType::Vector], 0, ValueType::Vector, false), function!("sinh", vec![ValueType::Vector], 0, ValueType::Vector, false), diff --git a/src/parser/parse.rs b/src/parser/parse.rs index 3d706d5..360ce74 100644 --- a/src/parser/parse.rs +++ b/src/parser/parse.rs @@ -2271,14 +2271,17 @@ mod tests { Expr::from(VectorSelector::from("bar")), ) }), + // start() and end() are also functions (Prometheus conformance). + ( + "start()", + Expr::new_call(get_function("start").unwrap(), FunctionArgs::empty_args()), + ), + ( + "end()", + Expr::new_call(get_function("end").unwrap(), FunctionArgs::empty_args()), + ), ]; assert_cases(Case::new_result_cases(cases)); - - let cases = vec![ - ("start()", INVALID_QUERY_INFO), - ("end()", INVALID_QUERY_INFO), - ]; - assert_cases(Case::new_fail_cases(cases)); } #[test] diff --git a/src/parser/promql.y b/src/parser/promql.y index b50e4e5..ef2acb9 100644 --- a/src/parser/promql.y +++ b/src/parser/promql.y @@ -356,6 +356,14 @@ function_call -> Result: Some(func) => Expr::new_call(func, $2?) } } + | at_modifier_preprocessors function_call_body + { + let name = $1?.val; + match get_function(&name) { + None => Err(format!("unknown function with name '{name}'")), + Some(func) => Expr::new_call(func, $2?) + } + } ; function_call_body -> Result: From b3f637607e52bddde77f7ba1ddd8dbb84626a316 Mon Sep 17 00:00:00 2001 From: Ning Sun Date: Mon, 28 Sep 2026 11:27:11 +0800 Subject: [PATCH 08/11] ci: create issue when ci job finds function mismatches (#154) * ci: create issue when ci job finds function mismatches * ci: fix failure check * ci: only create issue on confirmed function mismatch Distinguish the mismatch exit status (1) from operational errors (2) in compare_functions.py, and gate the issue-creation step on the mismatch exit code instead of a generic failure(). This prevents a false "out of sync" issue (which would then suppress later genuine reports via dedup) when the comparison fails for operational reasons such as a network error fetching Prometheus functions.go. --------- Co-authored-by: Yingwen --- .github/workflows/function-comparison.yml | 63 ++++++++++++++++++++++- scripts/compare_functions.py | 18 +++++-- src/parser/function.rs | 18 +++++++ src/parser/parse.rs | 40 ++++++++++++++ 4 files changed, 135 insertions(+), 4 deletions(-) diff --git a/.github/workflows/function-comparison.yml b/.github/workflows/function-comparison.yml index 405c563..bb8db9a 100644 --- a/.github/workflows/function-comparison.yml +++ b/.github/workflows/function-comparison.yml @@ -16,10 +16,18 @@ name: Function Comparison on: + pull_request: + push: + branches: + - main schedule: - cron: '0 0 * * 0' workflow_dispatch: +permissions: + contents: read + issues: write + jobs: compare: name: Compare Functions @@ -31,4 +39,57 @@ jobs: with: python-version: '3.x' - name: Run function comparison - run: python scripts/compare_functions.py + id: compare + # explicit shell: 'bash' in GitHub Actions means + # `bash --noprofile --norc -eo pipefail`. `set +e` disables errexit + # so the script's exit code can be captured from PIPESTATUS (pipefail + # + `tee` would otherwise mask it) and exposed as a step output. + # Exit codes: 0 = in sync, 1 = mismatch, 2 = operational error. + shell: bash + run: | + set +e + python scripts/compare_functions.py | tee comparison.log + code=${PIPESTATUS[0]} + echo "exit_code=$code" >> "$GITHUB_OUTPUT" + exit "$code" + - name: Create issue on scheduled mismatch + # Only a confirmed mismatch (exit code 1) files an issue. Operational + # failures (exit code 2, e.g. unable to fetch Prometheus functions.go) + # still fail the job but must not create an "out of sync" issue that + # would then suppress a later genuine mismatch report via dedup. + # failure() overrides the implicit success() check after a mismatch. + if: failure() && github.event_name == 'schedule' && steps.compare.outputs.exit_code == '1' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + ISSUE_TITLE="Function comparison failed: Rust functions out of sync with Prometheus" + EXISTING=$(gh issue list \ + --repo "${{ github.repository }}" \ + --state open \ + --search "in:title \"${ISSUE_TITLE}\"" \ + --json number \ + --jq 'length') + if [ "$EXISTING" -gt 0 ]; then + echo "An open issue already exists, skipping creation." + exit 0 + fi + { + echo "The scheduled function comparison detected mismatches." + echo "" + echo "Rust functions in \`src/parser/function.rs\` are out of sync with" + echo "[Prometheus functions.go](https://github.com/prometheus/prometheus/blob/main/promql/parser/functions.go)." + echo "" + echo "See the [workflow run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details." + echo "" + echo "
Comparison output" + echo "" + echo '```' + cat comparison.log + echo '```' + echo "" + echo "
" + } > issue_body.md + gh issue create \ + --repo "${{ github.repository }}" \ + --title "$ISSUE_TITLE" \ + --body-file issue_body.md diff --git a/scripts/compare_functions.py b/scripts/compare_functions.py index 4652320..0356e8d 100755 --- a/scripts/compare_functions.py +++ b/scripts/compare_functions.py @@ -2,6 +2,11 @@ """ Script to compare Prometheus Go functions.go with Rust functions.rs Ensures Rust functions are complete and consistent with Go version. + +Exit codes: + 0 - success, functions are in sync + 1 - mismatch detected (Rust functions out of sync with Prometheus) + 2 - operational error (e.g. network failure, file read error) """ import re @@ -9,6 +14,13 @@ from typing import Dict, List, Optional, Tuple from dataclasses import dataclass +# Exit code for a confirmed mismatch between Go and Rust functions. +EXIT_MISMATCH = 1 +# Exit code for operational errors that are NOT mismatches (fetch/read +# failures, unexpected exceptions). The CI workflow distinguishes these so +# it only files an "out of sync" issue for a real mismatch. +EXIT_ERROR = 2 + @dataclass class FunctionDef: @@ -185,7 +197,7 @@ def main(): go_content = result.stdout except Exception as e: print(f"Error fetching Go file: {e}") - sys.exit(1) + sys.exit(EXIT_ERROR) # Read Rust functions.rs rust_file = "src/parser/function.rs" @@ -196,7 +208,7 @@ def main(): rust_content = f.read() except Exception as e: print(f"Error reading Rust file: {e}") - sys.exit(1) + sys.exit(EXIT_ERROR) # Parse both files go_functions = parse_go_functions(go_content) @@ -270,7 +282,7 @@ def main(): sys.exit(0) else: print("\nāŒ Issues found - please review and fix") - sys.exit(1) + sys.exit(EXIT_MISMATCH) if __name__ == "__main__": diff --git a/src/parser/function.rs b/src/parser/function.rs index 4d9a717..559c603 100644 --- a/src/parser/function.rs +++ b/src/parser/function.rs @@ -402,6 +402,13 @@ lazy_static! { ValueType::Vector, false ), + function!( + "integral", + vec![ValueType::Matrix, ValueType::Scalar], + 1, + ValueType::Vector, + true + ), function!( "irate", vec![ValueType::Matrix], @@ -729,6 +736,17 @@ mod tests { assert_eq!(rate.variadic, 0); assert!(!rate.experimental); + // integral was added upstream after v3.8; it is experimental and takes + // an optional scalar "strategy" argument (0=left, 1=right, 2=trapezoid). + let integral = get_function("integral").unwrap(); + assert_eq!( + integral.arg_types, + vec![ValueType::Matrix, ValueType::Scalar] + ); + assert_eq!(integral.variadic, 1); + assert_eq!(integral.return_type, ValueType::Vector); + assert!(integral.experimental); + for func_name in ["max_of", "min_of"] { let func = get_function(func_name).unwrap(); assert_eq!(func.arg_types, vec![ValueType::Scalar, ValueType::Scalar]); diff --git a/src/parser/parse.rs b/src/parser/parse.rs index 360ce74..339e644 100644 --- a/src/parser/parse.rs +++ b/src/parser/parse.rs @@ -1368,6 +1368,30 @@ mod tests { FunctionArgs::new_args(Expr::from(1.0)).append_args(Expr::from(2.0)), ) }), + ("integral(some_metric[5m])", { + Expr::new_matrix_selector( + Expr::from(VectorSelector::from("some_metric")), + duration::MINUTE_DURATION * 5, + ) + .and_then(|ex| { + Expr::new_call( + get_function("integral").unwrap(), + FunctionArgs::new_args(ex), + ) + }) + }), + ("integral(some_metric[5m], 1)", { + Expr::new_matrix_selector( + Expr::from(VectorSelector::from("some_metric")), + duration::MINUTE_DURATION * 5, + ) + .and_then(|ex| { + Expr::new_call( + get_function("integral").unwrap(), + FunctionArgs::new_args(ex).append_args(Expr::from(1.0)), + ) + }) + }), ("double_exponential_smoothing(some_metric[5m], 0.5, 0.1)", { Expr::new_matrix_selector( Expr::from(VectorSelector::from("some_metric")), @@ -1862,6 +1886,22 @@ mod tests { "rate(some_metric)", "expected type matrix in call to function 'rate', got vector", ), + ( + "integral(some_metric)", + "expected type matrix in call to function 'integral', got vector", + ), + ( + "integral()", + "expected at least 1 argument(s) in call to 'integral', got 0", + ), + ( + "integral(some_metric[5m], 1, 2)", + "expected at most 2 argument(s) in call to 'integral', got 3", + ), + ( + r#"integral(some_metric[5m], "strategy")"#, + "expected type scalar in call to function 'integral', got string", + ), ( "ln(1)", "expected type vector in call to function 'ln', got scalar", From ae913aa80efabdcd3267454c14525706cbca7cdb Mon Sep 17 00:00:00 2001 From: Yingwen Date: Mon, 28 Sep 2026 11:40:30 +0800 Subject: [PATCH 09/11] chore: bump version to 0.11.0 (#156) Signed-off-by: evenyag --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index e44ed39..cb916f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ name = "promql-parser" readme = "README.md" description = "Parse PromQL query into AST" repository = "https://github.com/GreptimeTeam/promql-parser" -version = "0.10.0" +version = "0.11.0" edition = "2021" authors = ["The GreptimeDB Project Developers"] keywords = ["prometheus", "promql", "parser"] From 86b4019d4f29691b12b85194b2b4d48503695974 Mon Sep 17 00:00:00 2001 From: Ben Ye Date: Tue, 20 Oct 2026 20:26:34 -0700 Subject: [PATCH 10/11] fix: avoid stack overflow by making lexer iteration non-recursive (#157) `::next` tail-calls itself for every state transition that does not yield a lexeme, and rustc does not guarantee tail-call elimination. Two of those transitions form cycles that can repeat without producing a lexeme, so the recursion depth grew with the length of the input instead of staying constant: - `String` -> `Escape` -> `String`, one frame pair per escape sequence within a single quoted string; - `Start` -> `LineComment`/`Space` -> `Start`, one frame pair per comment line or whitespace run. A stack overflow aborts the process and cannot be caught, so a single long query takes down the whole program rather than returning a parse error. This was reached in production by a 0.8 MB PromQL query whose label matcher held a large regex alternation with 29,416 escape sequences: it exhausted an 8 MB stack inside one `next` call. Rewrite the function as a loop. The state machine, the order of the match arms and the values returned are unchanged, so this only affects how much stack the iteration uses. With the loop, the same 0.8 MB query parses on a 1 MB stack. Add a regression test that lexes both cycles on a deliberately small (256 KB) stack, so a return to per-item recursion is caught rather than hidden by a large default stack. --- src/parser/lex.rs | 78 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 72 insertions(+), 6 deletions(-) diff --git a/src/parser/lex.rs b/src/parser/lex.rs index 527e7c4..5b21c4c 100644 --- a/src/parser/lex.rs +++ b/src/parser/lex.rs @@ -678,12 +678,20 @@ impl Iterator for Lexer { type Item = Result; fn next(&mut self) -> Option { - self.shift(); - match &self.state { - State::Lexeme(token_id) => Some(Ok(self.lexeme(*token_id))), - State::Err(info) => Some(Err(info.clone())), - State::End => None, - _ => self.next(), + // Iterative rather than recursive: the skip arm below is a tail call, + // and rustc does not guarantee tail-call elimination. Some state + // transitions form cycles that yield no lexeme (`String` -> `Escape` + // -> `String`, `Start` -> `LineComment`/`Space` -> `Start`), so + // recursing there grew the stack with the length of the input and + // overflowed on long queries. + loop { + self.shift(); + match &self.state { + State::Lexeme(token_id) => return Some(Ok(self.lexeme(*token_id))), + State::Err(info) => return Some(Err(info.clone())), + State::End => return None, + _ => continue, + } } } } @@ -1438,6 +1446,64 @@ mod tests { assert_matches(cases); } + /// Runs `f` on a thread with a deliberately small stack, so that a + /// regression into per-input-item recursion is caught rather than + /// absorbed by a large default stack. + fn assert_lexes_on_small_stack(input: String, expected: Vec) { + const STACK_SIZE: usize = 256 * 1024; + + let handle = std::thread::Builder::new() + .stack_size(STACK_SIZE) + .spawn(move || { + let actual: Vec> = Lexer::new(&input) + .filter(|r| !matches!(r, Ok(l) if l.tok_id() == T_EOF)) + .collect(); + let expected: Vec> = expected + .into_iter() + .map(|(token_id, start, len)| Ok(LexemeType::new(token_id, start, len))) + .collect(); + assert_eq!(expected, actual, "\n: {}", input.len()); + }) + .expect("failed to spawn lexer thread"); + + handle.join().expect("lexer thread panicked or overflowed"); + } + + /// `::next` must not consume stack proportional to its + /// input. + /// + /// `next` used to tail-call itself for every state transition that does + /// not yield a lexeme, and rustc does not guarantee tail-call + /// elimination. Two of those transitions form cycles that can repeat + /// without producing a lexeme, so the recursion depth grew with the length + /// of the input rather than staying constant: + /// + /// - `String` -> `Escape` -> `String`, one frame pair per escape sequence + /// inside a single quoted string; + /// - `Start` -> `LineComment`/`Space` -> `Start`, one frame pair per + /// comment line or whitespace run. + /// + /// A 0.8 MB production query whose label matcher held a large regex with + /// 29,416 escape sequences exhausted an 8 MB stack this way, aborting the + /// process. A stack overflow cannot be caught, so this is not recoverable + /// by the caller. + #[test] + fn test_next_does_not_recurse_on_input_length() { + // One escape sequence per repetition, none of which yields a lexeme + // until the closing quote. The whole input lexes to a single string. + const ESCAPES: usize = 50_000; + let escaped = r"\\.".repeat(ESCAPES); + let input = format!("\"{escaped}\""); + let len = input.len(); + assert_lexes_on_small_stack(input, vec![(T_STRING, 0, len)]); + + // Comment lines and whitespace runs yield no lexeme at all, so the + // entire input is consumed within one `next` call. + const LINES: usize = 50_000; + let input = "# c\n \t\n".repeat(LINES); + assert_lexes_on_small_stack(input, vec![]); + } + #[test] fn test_is_alpha() { assert!(is_alpha('_')); From 5af6b808a905837273dc495550de688be5c9473d Mon Sep 17 00:00:00 2001 From: Yingwen Date: Wed, 21 Oct 2026 12:01:25 +0800 Subject: [PATCH 11/11] chore: bump version to 0.11.1 (#158) Signed-off-by: evenyag --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index cb916f3..4d7e860 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ name = "promql-parser" readme = "README.md" description = "Parse PromQL query into AST" repository = "https://github.com/GreptimeTeam/promql-parser" -version = "0.11.0" +version = "0.11.1" edition = "2021" authors = ["The GreptimeDB Project Developers"] keywords = ["prometheus", "promql", "parser"]