From 3f10427f52f8c32f80cc03ebd3a2bb8bd910b0d1 Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Wed, 9 Jul 2025 14:47:02 +0300 Subject: [PATCH] core: Fix resolve_function() error messages We need to return the original function name, not normalized one to be compatible with SQLite. Spotted by SQLite TCL tests. --- Cargo.lock | 2 ++ core/function.rs | 3 +- core/translate/expr.rs | 5 ++- core/translate/planner.rs | 7 ++-- core/translate/select.rs | 74 +++++++++++++++++---------------------- 5 files changed, 41 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b7c596247..3eab45b96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2222,6 +2222,8 @@ dependencies = [ "once_cell", "proc-macro2", "quote", + "regex", + "semver", "syn 2.0.100", ] diff --git a/core/function.rs b/core/function.rs index 7827e6307..58cf87b1e 100644 --- a/core/function.rs +++ b/core/function.rs @@ -616,7 +616,8 @@ impl Func { } } pub fn resolve_function(name: &str, arg_count: usize) -> Result { - match name { + let normalized_name = crate::util::normalize_ident(name); + match normalized_name.as_str() { "avg" => { if arg_count != 1 { crate::bail_parse_error!("wrong number of arguments to function {}()", name) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index fe78275c7..a482d119b 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -9,7 +9,7 @@ use crate::function::JsonFunc; use crate::function::{Func, FuncCtx, MathFuncArity, ScalarFunc, VectorFunc}; use crate::functions::datetime; use crate::schema::{Affinity, Table, Type}; -use crate::util::{exprs_are_equivalent, normalize_ident, parse_numeric_literal}; +use crate::util::{exprs_are_equivalent, parse_numeric_literal}; use crate::vdbe::builder::CursorKey; use crate::vdbe::{ builder::ProgramBuilder, @@ -680,8 +680,7 @@ pub fn translate_expr( order_by: _, } => { let args_count = if let Some(args) = args { args.len() } else { 0 }; - let func_name = normalize_ident(name.0.as_str()); - let func_type = resolver.resolve_function(&func_name, args_count); + let func_type = resolver.resolve_function(&name.0, args_count); if func_type.is_none() { crate::bail_parse_error!("unknown function {}", name.0); diff --git a/core/translate/planner.rs b/core/translate/planner.rs index 56f51bc5c..00ebaf897 100644 --- a/core/translate/planner.rs +++ b/core/translate/planner.rs @@ -51,8 +51,7 @@ pub fn resolve_aggregates( } else { 0 }; - match Func::resolve_function(normalize_ident(name.0.as_str()).as_str(), args_count) - { + match Func::resolve_function(&name.0, args_count) { Ok(Func::Agg(f)) => { let distinctness = Distinctness::from_ast(distinctness.as_ref()); if !schema.indexes_enabled() && distinctness.is_distinct() { @@ -84,9 +83,7 @@ pub fn resolve_aggregates( } } Expr::FunctionCallStar { name, .. } => { - if let Ok(Func::Agg(f)) = - Func::resolve_function(normalize_ident(name.0.as_str()).as_str(), 0) - { + if let Ok(Func::Agg(f)) = Func::resolve_function(&name.0, 0) { aggs.push(Aggregate { func: f, args: vec![], diff --git a/core/translate/select.rs b/core/translate/select.rs index df968b16a..4e67f4659 100644 --- a/core/translate/select.rs +++ b/core/translate/select.rs @@ -340,10 +340,7 @@ fn prepare_one_select_plan( if distinctness.is_distinct() && args_count != 1 { crate::bail_parse_error!("DISTINCT aggregate functions must have exactly one argument"); } - match Func::resolve_function( - normalize_ident(name.0.as_str()).as_str(), - args_count, - ) { + match Func::resolve_function(&name.0, args_count) { Ok(Func::Agg(f)) => { let agg_args = match (args, &f) { (None, crate::function::AggFunc::Count0) => { @@ -442,49 +439,44 @@ fn prepare_one_select_plan( ast::Expr::FunctionCallStar { name, filter_over: _, - } => { - match Func::resolve_function( - normalize_ident(name.0.as_str()).as_str(), - 0, - ) { - Ok(Func::Agg(f)) => { - let agg = Aggregate { - func: f, - args: vec![ast::Expr::Literal(ast::Literal::Numeric( - "1".to_string(), - ))], - original_expr: expr.clone(), - distinctness: Distinctness::NonDistinct, - }; - aggregate_expressions.push(agg.clone()); - plan.result_columns.push(ResultSetColumn { - alias: maybe_alias.as_ref().map(|alias| match alias { - ast::As::Elided(alias) => alias.0.clone(), - ast::As::As(alias) => alias.0.clone(), - }), - expr: expr.clone(), - contains_aggregates: true, - }); + } => match Func::resolve_function(&name.0, 0) { + Ok(Func::Agg(f)) => { + let agg = Aggregate { + func: f, + args: vec![ast::Expr::Literal(ast::Literal::Numeric( + "1".to_string(), + ))], + original_expr: expr.clone(), + distinctness: Distinctness::NonDistinct, + }; + aggregate_expressions.push(agg.clone()); + plan.result_columns.push(ResultSetColumn { + alias: maybe_alias.as_ref().map(|alias| match alias { + ast::As::Elided(alias) => alias.0.clone(), + ast::As::As(alias) => alias.0.clone(), + }), + expr: expr.clone(), + contains_aggregates: true, + }); + } + Ok(_) => { + crate::bail_parse_error!( + "Invalid aggregate function: {}", + name.0 + ); + } + Err(e) => match e { + crate::LimboError::ParseError(e) => { + crate::bail_parse_error!("{}", e); } - Ok(_) => { + _ => { crate::bail_parse_error!( "Invalid aggregate function: {}", name.0 ); } - Err(e) => match e { - crate::LimboError::ParseError(e) => { - crate::bail_parse_error!("{}", e); - } - _ => { - crate::bail_parse_error!( - "Invalid aggregate function: {}", - name.0 - ); - } - }, - } - } + }, + }, expr => { let contains_aggregates = resolve_aggregates(schema, expr, &mut aggregate_expressions)?;