From d667728fdeb5a55e63665f070453b4649801af60 Mon Sep 17 00:00:00 2001 From: shuiyisong <113876041+shuiyisong@users.noreply.github.com> Date: Mon, 13 Jul 2026 11:27:06 +0800 Subject: [PATCH] chore!: update promql-parser to v0.10.0, remove `holt_winters` (#8457) * chore: update promql parser and fix compile Signed-off-by: shuiyisong * fix: sqlness Signed-off-by: shuiyisong * fix: cr issue Signed-off-by: shuiyisong * fix: cr issue Signed-off-by: shuiyisong --------- Signed-off-by: shuiyisong --- Cargo.lock | 8 +-- Cargo.toml | 2 +- src/query/src/parser.rs | 49 ++++++++++++------- src/query/src/promql/planner.rs | 41 ++++++++++++++++ .../standalone/common/promql/functions.result | 10 ---- .../standalone/common/promql/functions.sql | 4 -- 6 files changed, 76 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e00e30e28a..75ba5ce410 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11038,9 +11038,9 @@ dependencies = [ [[package]] name = "promql-parser" -version = "0.7.3" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f514ab521bb2b4ce26aca29bf133498b2a2651b785d03a262bbf5772329c279f" +checksum = "9e8c90c956be4237a93be6f2298302b56974ef862ab11e0f98869d1f27c69f88" dependencies = [ "cfgrammar", "chrono", @@ -11183,7 +11183,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a56d757972c98b346a9b766e3f02746cde6dd1cd1d1d563472929fdd74bec4d" dependencies = [ "anyhow", - "itertools 0.11.0", + "itertools 0.14.0", "proc-macro2", "quote", "syn 2.0.117", @@ -11196,7 +11196,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9120690fafc389a67ba3803df527d0ec9cbbc9cc45e4cc20b332996dfb672425" dependencies = [ "anyhow", - "itertools 0.11.0", + "itertools 0.14.0", "proc-macro2", "quote", "syn 2.0.117", diff --git a/Cargo.toml b/Cargo.toml index 5ee60c450e..fd878c7c0a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -198,7 +198,7 @@ paste = "1.0" pin-project = "1.0" pretty_assertions = "1.4.0" prometheus = { version = "0.14", features = ["process"] } -promql-parser = { version = "0.7.3", features = ["ser"] } +promql-parser = { version = "0.10.0", features = ["ser"] } prost = { version = "0.14", features = ["no-recursion-limit"] } prost-types = "0.14" raft-engine = { version = "0.4.1", default-features = false } diff --git a/src/query/src/parser.rs b/src/query/src/parser.rs index 53f24557c7..87b827e0cc 100644 --- a/src/query/src/parser.rs +++ b/src/query/src/parser.rs @@ -52,45 +52,37 @@ pub enum QueryStatement { } impl QueryStatement { - pub fn post_process(&self, params: HashMap) -> Result { + pub fn post_process(self, params: HashMap) -> Result { match self { QueryStatement::Sql(_) => UnimplementedSnafu { operation: "sql post process", } .fail(), - QueryStatement::Promql(eval_stmt, alias) => { + QueryStatement::Promql(mut eval_stmt, alias) => { let node_name = match params.get("name") { Some(name) => name.as_str(), None => "", }; - let extension_node = Self::create_extension_node(node_name, &eval_stmt.expr); - Ok(QueryStatement::Promql( - EvalStmt { - expr: Extension(extension_node.unwrap()), - start: eval_stmt.start, - end: eval_stmt.end, - interval: eval_stmt.interval, - lookback_delta: eval_stmt.lookback_delta, - }, - alias.clone(), - )) + let extension_node = Self::create_extension_node(node_name, eval_stmt.expr); + eval_stmt.expr = Extension(extension_node.unwrap()); + Ok(QueryStatement::Promql(eval_stmt, alias)) } } } - fn create_extension_node(node_name: &str, expr: &Expr) -> Option { + fn create_extension_node(node_name: &str, expr: Expr) -> Option { match node_name { ANALYZE_NODE_NAME => Some(NodeExtension { - expr: Arc::new(AnalyzeExpr { expr: expr.clone() }), + expr: Arc::new(AnalyzeExpr { expr }), }), ANALYZE_VERBOSE_NODE_NAME => Some(NodeExtension { - expr: Arc::new(AnalyzeVerboseExpr { expr: expr.clone() }), + expr: Arc::new(AnalyzeVerboseExpr { expr }), }), EXPLAIN_NODE_NAME => Some(NodeExtension { - expr: Arc::new(ExplainExpr { expr: expr.clone() }), + expr: Arc::new(ExplainExpr { expr }), }), EXPLAIN_VERBOSE_NODE_NAME => Some(NodeExtension { - expr: Arc::new(ExplainVerboseExpr { expr: expr.clone() }), + expr: Arc::new(ExplainVerboseExpr { expr }), }), _ => None, } @@ -204,9 +196,10 @@ impl QueryLanguageParser { } pub(crate) fn apply_alias_extension(mut eval_stmt: EvalStmt, alias: &str) -> EvalStmt { + let expr = eval_stmt.expr; eval_stmt.expr = Extension(NodeExtension { expr: Arc::new(AliasExpr { - expr: eval_stmt.expr.clone(), + expr, alias: alias.to_string(), }), }); @@ -263,6 +256,14 @@ macro_rules! define_node_ast_extension { fn children(&self) -> &[Expr] { std::slice::from_ref(&self.expr) } + + fn with_new_children(&self, children: Vec) -> Arc { + let mut iter = children.into_iter(); + match (iter.next(), iter.next()) { + (Some(expr), None) => Arc::new($name_expr { expr }), + _ => Arc::new(self.clone()), + } + } } #[allow(rustdoc::broken_intra_doc_links)] @@ -313,6 +314,16 @@ impl ExtensionExpr for AliasExpr { fn children(&self) -> &[Expr] { std::slice::from_ref(&self.expr) } + fn with_new_children(&self, children: Vec) -> Arc { + let mut iter = children.into_iter(); + match (iter.next(), iter.next()) { + (Some(expr), None) => Arc::new(Self { + expr, + alias: self.alias.clone(), + }), + _ => Arc::new(self.clone()), + } + } } #[derive(Debug, Clone)] pub struct Alias { diff --git a/src/query/src/promql/planner.rs b/src/query/src/promql/planner.rs index c2cd6316ad..61913156cf 100644 --- a/src/query/src/promql/planner.rs +++ b/src/query/src/promql/planner.rs @@ -229,6 +229,8 @@ impl IslandExpr { !modifier.return_bool && modifier.matching.is_none() && matches!(modifier.card, VectorMatchCardinality::OneToOne) + && modifier.fill_values.lhs.is_none() + && modifier.fill_values.rhs.is_none() }) => { let lhs = Self::try_new(lhs, env)?; @@ -1093,6 +1095,18 @@ impl PromPlanner { query_engine_state: &QueryEngineState, binary_expr: &PromBinaryExpr, ) -> Result { + // promql-parser accepts fill modifiers, but Greptime does not implement the + // required outer joins and missing-value substitution. Reject them before the + // binary-island fast path so they cannot silently behave like normal inner joins. + if let Some(modifier) = &binary_expr.modifier { + ensure!( + modifier.fill_values.lhs.is_none() && modifier.fill_values.rhs.is_none(), + UnsupportedExprSnafu { + name: "PromQL fill modifiers" + } + ); + } + if let Some(plan) = self.try_plan_binary_island(binary_expr).await? { return Ok(plan); } @@ -5542,6 +5556,33 @@ mod test { ); } + #[tokio::test] + async fn reject_binary_fill_modifiers() { + let state = build_query_engine_state(); + + for query in [ + "some_metric + fill(0) some_alt_metric", + "some_metric + fill_left(0) some_alt_metric", + "some_metric + fill_right(0) some_alt_metric", + "(some_metric + fill(0) some_alt_metric) + some_metric", + ] { + let eval_stmt = build_eval_stmt(query); + let table_provider = build_test_table_provider(&[], 0, 0).await; + let err = PromPlanner::stmt_to_plan(table_provider, &eval_stmt, &state) + .await + .unwrap_err(); + + assert!( + matches!( + &err, + crate::promql::error::Error::UnsupportedExpr { name, .. } + if name == "PromQL fill modifiers" + ), + "{err}" + ); + } + } + #[tokio::test] async fn timestamp_binary_join_falls_back_when_tsid_is_projected_out() { for query in [ diff --git a/tests/cases/standalone/common/promql/functions.result b/tests/cases/standalone/common/promql/functions.result index 02ff599ced..325fbccd17 100644 --- a/tests/cases/standalone/common/promql/functions.result +++ b/tests/cases/standalone/common/promql/functions.result @@ -62,16 +62,6 @@ tql eval (10, 10, '1s') double_exponential_smoothing(prom_series[10s], 0.4 + 0.1 | 1970-01-01T00:00:10 | 47.0806953125 | p | +---------------------+------------------------------------------------------------------------------------------+------+ --- holt_winters (backward compatibility) --- SQLNESS SORT_RESULT 3 1 -tql eval (10, 10, '1s') holt_winters(prom_series[10s], 0.4 + 0.1, 0.1); - -+---------------------+------------------------------------------------------------------------------------------+------+ -| ts | prom_double_exponential_smoothing(ts_range,val,Float64(0.4) + Float64(0.1),Float64(0.1)) | host | -+---------------------+------------------------------------------------------------------------------------------+------+ -| 1970-01-01T00:00:10 | 47.0806953125 | p | -+---------------------+------------------------------------------------------------------------------------------+------+ - DROP TABLE prom_series; Affected Rows: 0 diff --git a/tests/cases/standalone/common/promql/functions.sql b/tests/cases/standalone/common/promql/functions.sql index 6657fc49e8..0a257f29da 100644 --- a/tests/cases/standalone/common/promql/functions.sql +++ b/tests/cases/standalone/common/promql/functions.sql @@ -34,10 +34,6 @@ tql eval (3, 3, '1s') predict_linear(prom_series[3s], 40 + 2); -- SQLNESS SORT_RESULT 3 1 tql eval (10, 10, '1s') double_exponential_smoothing(prom_series[10s], 0.4 + 0.1, 0.1); --- holt_winters (backward compatibility) --- SQLNESS SORT_RESULT 3 1 -tql eval (10, 10, '1s') holt_winters(prom_series[10s], 0.4 + 0.1, 0.1); - DROP TABLE prom_series; CREATE TABLE