fix(functions): add hints for mismatched function args (#19735)

* fix(functions): add hints for mismatched args (#14026)

* fix(functions): preserve mismatch error codes

* fix(expression): avoid false function mismatch hints

* test: align pretty_error cluster expectation

* fix(expression): narrow mismatch hint suppression

* test: refresh pretty_error hint output

* test: align pretty_error standalone output

* fix(expression): simplify cast mismatch hints

* Revert "fix(expression): simplify cast mismatch hints"

This reverts commit 4fdb604.

* fix(expression): concretize mismatch hint candidates

* test: refresh pretty_error concrete candidates

Author: KKould

src/query/expression/src/evaluator.rs

@@ -34,6 +34,7 @@ use crate::expr::*;
 use crate::expression::Expr;
 use crate::function::EvalContext;
 use crate::type_check::check_function;
+use crate::type_check::format_function_argument_mismatch_hint;
 use crate::type_check::get_simple_cast_function;
 use crate::types::BooleanType;
 use crate::types::DataType;
@@ -293,10 +294,21 @@ impl<'a> Evaluator<'a> {
             child_option.strict_eval = false;
         }
 
-        let args = args
-            .iter()
-            .map(|expr| self.partial_run(expr, validity.clone(), &mut child_option))
-            .collect::<Result<Vec<_>>>()?;
+        let mut args_value = Vec::with_capacity(args.len());
+        for expr in args {
+            match self.partial_run(expr, validity.clone(), &mut child_option) {
+                Ok(value) => args_value.push(value),
+                Err(err) => {
+                    return Err(self.attach_function_argument_mismatch_hint(
+                        err,
+                        &function.signature.name,
+                        id.params(),
+                        args,
+                    ));
+                }
+            }
+        }
+        let args = args_value;
 
         assert!(
             args.iter()
@@ -342,6 +354,37 @@ impl<'a> Evaluator<'a> {
         Ok(result)
     }
 
+    fn attach_function_argument_mismatch_hint(
+        &self,
+        err: ErrorCode,
+        name: &str,
+        params: &[Scalar],
+        args: &[Expr],
+    ) -> ErrorCode {
+        let mut has_top_level_cast = false;
+        let hint_args = args
+            .iter()
+            .map(|arg| match arg {
+                Expr::Cast(Cast { expr, .. }) => {
+                    has_top_level_cast = true;
+                    expr.as_ref().clone()
+                }
+                _ => arg.clone(),
+            })
+            .collect::<Vec<_>>();
+        if !has_top_level_cast {
+            return err;
+        }
+
+        let Some(hint) =
+            format_function_argument_mismatch_hint(name, params, &hint_args, self.fn_registry)
+        else {
+            return err;
+        };
+
+        err.add_message_back(format!("\n\nhint: {hint}"))
+    }
+
     pub fn run_cast(
         &self,
         span: Span,

src/query/expression/src/type_check.rs

@@ -41,6 +41,8 @@ use crate::types::decimal::DecimalSize;
 use crate::types::i256;
 use crate::visit_expr;
 
+const MAX_FUNCTION_MISMATCH_HINT_CANDIDATES: usize = 10;
+
 #[recursive::recursive]
 pub fn check<Index: ColumnIndex>(
     expr: &RawExpr<Index>,
@@ -392,22 +394,7 @@ pub fn check_function<Index: ColumnIndex>(
         return Ok(checked_candidates.pop().unwrap().0);
     }
 
-    let mut msg = if params.is_empty() {
-        format!(
-            "no function matches signature `{name}({})`, you might need to add explicit type casts.",
-            args.iter()
-                .map(|arg| arg.data_type().to_string())
-                .join(", ")
-        )
-    } else {
-        format!(
-            "no function matches signature `{name}({})({})`, you might need to add explicit type casts.",
-            params.iter().join(", "),
-            args.iter()
-                .map(|arg| arg.data_type().to_string())
-                .join(", ")
-        )
-    };
+    let mut msg = format_no_matching_function_signature(name, params, args);
 
     if !candidates.is_empty() {
         let candidates_sig: Vec<_> = candidates
@@ -426,21 +413,128 @@ pub fn check_function<Index: ColumnIndex>(
             .map(|(sig, err)| format!("  {sig:<max_len$}  : {}", err.message()))
             .join("\n");
 
-        let shorten_msg = if candidates_len > take_len {
-            format!("\n... and {} more", candidates_len - take_len)
-        } else {
-            "".to_string()
-        };
         write!(
             &mut msg,
-            "\n\ncandidate functions:\n{candidates_fail_reason}{shorten_msg}",
+            "\n\ncandidate functions:\n{candidates_fail_reason}",
         )
         .unwrap();
+        if candidates_len > take_len {
+            write!(&mut msg, "\n... and {} more", candidates_len - take_len).unwrap();
+        }
     };
 
     Err(ErrorCode::SemanticError(msg).set_span(span))
 }
 
+fn format_function_signature<Index: ColumnIndex>(
+    name: &str,
+    params: &[Scalar],
+    args: &[Expr<Index>],
+) -> String {
+    let args = args
+        .iter()
+        .map(|arg| arg.data_type().to_string())
+        .collect::<Vec<_>>();
+    format_function_signature_from_arg_types(name, params, &args)
+}
+
+fn format_function_signature_from_arg_types(
+    name: &str,
+    params: &[Scalar],
+    args: &[String],
+) -> String {
+    let args = args.iter().join(", ");
+    if params.is_empty() {
+        format!("{name}({args})")
+    } else {
+        format!("{name}({})({args})", params.iter().join(", "))
+    }
+}
+
+fn format_no_matching_function_signature<Index: ColumnIndex>(
+    name: &str,
+    params: &[Scalar],
+    args: &[Expr<Index>],
+) -> String {
+    format!(
+        "no function matches signature `{}`, you might need to add explicit type casts.",
+        format_function_signature(name, params, args)
+    )
+}
+
+pub fn format_function_argument_mismatch_hint<Index: ColumnIndex>(
+    name: &str,
+    params: &[Scalar],
+    args: &[Expr<Index>],
+    fn_registry: &FunctionRegistry,
+) -> Option<String> {
+    let name = fn_registry
+        .aliases
+        .get(name)
+        .map(|name| name.as_str())
+        .unwrap_or(name);
+
+    let candidates = fn_registry.search_candidates(name, params, args);
+    if candidates.is_empty() {
+        return None;
+    }
+
+    let auto_cast_rules = fn_registry.get_auto_cast_rules(name);
+    let dynamic_cast_rules = fn_registry.get_dynamic_cast_rules(name);
+    let mut concrete_candidates_sig = Vec::new();
+    for (_, func) in &candidates {
+        let Ok((checked_args, return_type, _)) = try_check_function(
+            args,
+            &func.signature,
+            auto_cast_rules,
+            &dynamic_cast_rules,
+            fn_registry,
+        ) else {
+            continue;
+        };
+
+        if checked_args == args {
+            return None;
+        }
+
+        concrete_candidates_sig.push(format!(
+            "{} :: {}",
+            format_function_signature(name, params, &checked_args),
+            return_type
+        ));
+    }
+
+    let mut msg = format_no_matching_function_signature(name, params, args);
+
+    let candidates_sig = if concrete_candidates_sig.is_empty() {
+        candidates
+            .iter()
+            .map(|(_, func)| func.signature.to_string())
+            .collect()
+    } else {
+        concrete_candidates_sig
+    };
+    let (mut candidates_sig, nullable_candidates_sig): (Vec<_>, Vec<_>) = candidates_sig
+        .into_iter()
+        .unique()
+        .partition(|sig| !sig.contains("NULL"));
+    candidates_sig.extend(nullable_candidates_sig);
+
+    let candidates_len = candidates_sig.len();
+    let take_len = candidates_len.min(MAX_FUNCTION_MISMATCH_HINT_CANDIDATES);
+    let candidates_sig = candidates_sig
+        .into_iter()
+        .take(take_len)
+        .map(|sig| format!("  {sig}"))
+        .join("\n");
+    write!(&mut msg, "\n\ncandidate functions:\n{candidates_sig}").unwrap();
+    if candidates_len > take_len {
+        write!(&mut msg, "\n... and {} more", candidates_len - take_len).unwrap();
+    }
+
+    Some(msg)
+}
+
 #[derive(Debug)]
 pub struct Substitution(pub HashMap<usize, DataType>);
 

src/query/functions/tests/it/scalars/testdata/array.txt

@@ -11,7 +11,13 @@ error:
   --> SQL:1:2
   |
 1 | ['a', 1]
-  |  ^^^ bad decimal literal: unexpected char while evaluating function `to_decimal(38, 5)('a')` in expr `CAST('a' AS Decimal(38, 5))`, during run expr: `array(CAST('a' AS Decimal(38, 5)), CAST(1 AS Decimal(38, 5)))`
+  |  ^^^ bad decimal literal: unexpected char while evaluating function `to_decimal(38, 5)('a')` in expr `CAST('a' AS Decimal(38, 5))`
+
+
+hint: no function matches signature `array(String, UInt8)`, you might need to add explicit type casts.
+
+candidate functions:
+  array(Decimal(38, 5), Decimal(38, 5)) :: Array(Decimal(38, 5)), during run expr: `array(CAST('a' AS Decimal(38, 5)), CAST(1 AS Decimal(38, 5)))`
 
 
 
@@ -157,7 +163,14 @@ error:
   --> SQL:1:8
   |
 1 | [1, 2]['a']
-  |        ^^^ invalid digit found in string while evaluating function `to_uint64('a')` in expr `CAST('a' AS UInt64)`, during run expr: `get(CAST(array(1, 2) AS Array(UInt8 NULL)), CAST('a' AS UInt64))`
+  |        ^^^ invalid digit found in string while evaluating function `to_uint64('a')` in expr `CAST('a' AS UInt64)`
+
+
+hint: no function matches signature `get(Array(UInt8), String)`, you might need to add explicit type casts.
+
+candidate functions:
+  get(Array(UInt8 NULL), UInt64) :: UInt8 NULL
+  get(Array(UInt8 NULL) NULL, UInt64 NULL) :: UInt8 NULL, during run expr: `get(CAST(array(1, 2) AS Array(UInt8 NULL)), CAST('a' AS UInt64))`
 
 
 
@@ -805,7 +818,14 @@ error:
   --> SQL:1:23
   |
 1 | array_concat([1,2,3], ['s', null])
-  |                       ^^^^^^^^^^^ bad decimal literal: unexpected char while evaluating function `to_decimal(38, 5)('s')` in expr `CAST(array(CAST('s' AS String NULL), CAST(NULL AS String NULL)) AS Array(Decimal(38, 5) NULL))`, during run expr: `array_concat(CAST(array(1, 2, 3) AS Array(Decimal(38, 5) NULL)), CAST(array(CAST('s' AS String NULL), CAST(NULL AS String NULL)) AS Array(Decimal(38, 5) NULL)))`
+  |                       ^^^^^^^^^^^ bad decimal literal: unexpected char while evaluating function `to_decimal(38, 5)('s')` in expr `CAST(array(CAST('s' AS String NULL), CAST(NULL AS String NULL)) AS Array(Decimal(38, 5) NULL))`
+
+
+hint: no function matches signature `array_concat(Array(UInt8), Array(String NULL))`, you might need to add explicit type casts.
+
+candidate functions:
+  array_concat(Array(Decimal(38, 5) NULL), Array(Decimal(38, 5) NULL)) :: Array(Decimal(38, 5) NULL)
+  array_concat(Array(Decimal(38, 5) NULL) NULL, Array(Decimal(38, 5) NULL) NULL) :: Array(Decimal(38, 5) NULL) NULL, during run expr: `array_concat(CAST(array(1, 2, 3) AS Array(Decimal(38, 5) NULL)), CAST(array(CAST('s' AS String NULL), CAST(NULL AS String NULL)) AS Array(Decimal(38, 5) NULL)))`
 
 
 
@@ -1058,7 +1078,20 @@ error:
   --> SQL:1:22
   |
 1 | array_indexof([1,2,3,'s'], 's')
-  |                      ^^^ bad decimal literal: unexpected char while evaluating function `to_decimal(38, 5)('s')` in expr `CAST('s' AS Decimal(38, 5))`, during run expr: `array_indexof(array(CAST(1 AS Decimal(38, 5)), CAST(2 AS Decimal(38, 5)), CAST(3 AS Decimal(38, 5)), CAST('s' AS Decimal(38, 5))), CAST('s' AS Decimal(38, 5)))`
+  |                      ^^^ bad decimal literal: unexpected char while evaluating function `to_decimal(38, 5)('s')` in expr `CAST('s' AS Decimal(38, 5))`
+
+
+hint: no function matches signature `array(UInt8, UInt8, UInt8, String)`, you might need to add explicit type casts.
+
+candidate functions:
+  array(Decimal(38, 5), Decimal(38, 5), Decimal(38, 5), Decimal(38, 5)) :: Array(Decimal(38, 5))
+
+
+hint: no function matches signature `array_indexof(Array(Decimal(38, 5)), String)`, you might need to add explicit type casts.
+
+candidate functions:
+  array_indexof(Array(Decimal(38, 5)), Decimal(38, 5)) :: UInt64
+  array_indexof(Array(Decimal(38, 5)) NULL, Decimal(38, 5) NULL) :: UInt64 NULL, during run expr: `array_indexof(array(CAST(1 AS Decimal(38, 5)), CAST(2 AS Decimal(38, 5)), CAST(3 AS Decimal(38, 5)), CAST('s' AS Decimal(38, 5))), CAST('s' AS Decimal(38, 5)))`
 
 
 

src/query/functions/tests/it/scalars/testdata/boolean.txt

@@ -65,15 +65,29 @@ error:
   --> SQL:1:1
   |
 1 | 'a' and 1
-  | ^^^ cannot parse to type `BOOLEAN` while evaluating function `to_boolean('a')` in expr `CAST('a' AS Boolean)`, during run expr: `(CAST('a' AS Boolean) AND CAST(1 AS Boolean))`
+  | ^^^ cannot parse to type `BOOLEAN` while evaluating function `to_boolean('a')` in expr `CAST('a' AS Boolean)`
+
+
+hint: no function matches signature `and(String, UInt8)`, you might need to add explicit type casts.
+
+candidate functions:
+  and(Boolean, Boolean) :: Boolean
+  and(Boolean NULL, Boolean NULL) :: Boolean NULL, during run expr: `(CAST('a' AS Boolean) AND CAST(1 AS Boolean))`
 
 
 
 error: 
   --> SQL:1:9
   |
 1 | NOT NOT 'a'
-  |         ^^^ cannot parse to type `BOOLEAN` while evaluating function `to_boolean('a')` in expr `CAST('a' AS Boolean)`, during run expr: `NOT NOT CAST('a' AS Boolean)`
+  |         ^^^ cannot parse to type `BOOLEAN` while evaluating function `to_boolean('a')` in expr `CAST('a' AS Boolean)`
+
+
+hint: no function matches signature `not(String)`, you might need to add explicit type casts.
+
+candidate functions:
+  not(Boolean) :: Boolean
+  not(Boolean NULL) :: Boolean NULL, during run expr: `NOT NOT CAST('a' AS Boolean)`
 
 
 

src/query/functions/tests/it/scalars/testdata/geo_h3.txt

@@ -791,7 +791,14 @@ error:
   --> SQL:1:14
   |
 1 | h3_to_string('8d11aa6a38826ff')
-  |              ^^^^^^^^^^^^^^^^^ invalid digit found in string while evaluating function `to_uint64('8d11aa6a38826ff')` in expr `CAST('8d11aa6a38826ff' AS UInt64)`, during run expr: `h3_to_string(CAST('8d11aa6a38826ff' AS UInt64))`
+  |              ^^^^^^^^^^^^^^^^^ invalid digit found in string while evaluating function `to_uint64('8d11aa6a38826ff')` in expr `CAST('8d11aa6a38826ff' AS UInt64)`
+
+
+hint: no function matches signature `h3_to_string(String)`, you might need to add explicit type casts.
+
+candidate functions:
+  h3_to_string(UInt64) :: String
+  h3_to_string(UInt64 NULL) :: String NULL, during run expr: `h3_to_string(CAST('8d11aa6a38826ff' AS UInt64))`