fix(sql): improve table function named arg hints (#19751)

fix(sql): improve table function named arg hints (#12691)

Author: KKould

src/query/sql/src/planner/binder/bind_table_reference/bind_obfuscate.rs

@@ -37,6 +37,7 @@ use crate::BindContext;
 use crate::ScalarBinder;
 use crate::binder::Binder;
 use crate::binder::table_args::bind_table_args;
+use crate::binder::table_args::malformed_named_table_arg_error;
 use crate::binder::util::TableIdentifier;
 use crate::optimizer::ir::SExpr;
 
@@ -47,20 +48,32 @@ impl Binder {
         params: &[Expr],
         named_params: &[(Identifier, Expr)],
     ) -> Result<(SExpr, BindContext)> {
-        let param = match params {
-            [] => Err(None),
-            [param @ Expr::ColumnRef { .. }] => Ok(param.clone()),
-            _ => Err(params[0].span()),
+        let (param, extra_params) = match params {
+            [] => (Err(None), &[][..]),
+            [param @ Expr::ColumnRef { .. }] => (Ok(param.clone()), &[][..]),
+            [param, extra_params @ ..] => (Err(param.span()), extra_params),
         };
 
+        if let Some(extra_param) = extra_params.first() {
+            if let Some(err) = malformed_named_table_arg_error("obfuscate", extra_param) {
+                return Err(err);
+            }
+
+            return Err(ErrorCode::InvalidArgument(
+                "The `OBFUSCATE` function accepts one table_name positional argument and an optional `seed => ...` named argument.",
+            )
+            .set_span(extra_param.span()));
+        }
+
         let mut scalar_binder = ScalarBinder::new(
             bind_context,
             self.ctx.clone(),
             &self.name_resolution_ctx,
             self.metadata.clone(),
             &[],
         );
-        let mut named_args = bind_table_args(&mut scalar_binder, &[], named_params, &None)?.named;
+        let mut named_args =
+            bind_table_args("obfuscate", &mut scalar_binder, &[], named_params, &None)?.named;
         let seed = match named_args.remove("seed") {
             Some(v) => u64_value(&v).ok_or(ErrorCode::BadArguments("invalid seed"))?,
             None => {

src/query/sql/src/planner/binder/bind_table_reference/bind_table_function.rs

@@ -137,6 +137,7 @@ impl Binder {
             &[],
         );
         let table_args = bind_table_args(
+            &func_name.name,
             &mut scalar_binder,
             params,
             named_params,

src/query/sql/src/planner/binder/table_args.rs

@@ -15,6 +15,9 @@
 use std::collections::HashMap;
 use std::sync::Arc;
 
+use databend_common_ast::ast::BinaryOperator;
+use databend_common_ast::ast::ColumnID;
+use databend_common_ast::ast::ColumnRef;
 use databend_common_ast::ast::Expr;
 use databend_common_ast::ast::Identifier;
 use databend_common_catalog::table_args::TableArgs;
@@ -142,15 +145,52 @@ fn try_fold_to_scalar(
     }
 }
 
+pub(crate) fn malformed_named_table_arg_error(func_name: &str, expr: &Expr) -> Option<ErrorCode> {
+    let Expr::BinaryOp {
+        op: BinaryOperator::Eq,
+        left,
+        ..
+    } = expr
+    else {
+        return None;
+    };
+
+    let Expr::ColumnRef {
+        column:
+            ColumnRef {
+                database: None,
+                table: None,
+                column: ColumnID::Name(name),
+            },
+        ..
+    } = left.as_ref()
+    else {
+        return None;
+    };
+
+    ErrorCode::SemanticError(format!(
+        "Table function '{}' uses `=>` for named arguments; replace `{} = ...` with `{} => ...`.",
+        func_name, name.name, name.name
+    ))
+    .set_span(expr.span())
+    .into()
+}
+
 pub fn bind_table_args(
+    func_name: &str,
     scalar_binder: &mut ScalarBinder<'_>,
     params: &[Expr],
     named_params: &[(Identifier, Expr)],
     subquery_executor: &Option<Arc<dyn QueryExecutor>>,
 ) -> Result<TableArgs> {
     let mut args = Vec::with_capacity(params.len());
     for arg in params.iter() {
-        args.push((scalar_binder.bind(arg)?.0, arg));
+        if let Some(err) = malformed_named_table_arg_error(func_name, arg) {
+            return Err(err);
+        }
+
+        let scalar = scalar_binder.bind(arg)?.0;
+        args.push((scalar, arg));
     }
 
     let mut named_args = Vec::with_capacity(named_params.len());

src/query/sql/tests/it/semantic/binder.rs

@@ -196,6 +196,18 @@ async fn test_binder_clauses_and_ordering() -> Result<()> {
             setup_sqls: &["CREATE TABLE t(number UInt64)"],
             sql: "SELECT number % 3 AS c1, sum(c1) FROM t GROUP BY number % 3",
         },
+        SqlTestCase {
+            name: "table_function_named_arguments_require_fat_arrow",
+            description: "A table function named argument written with '=' should produce a direct hint to use '=>'.",
+            setup_sqls: &[],
+            sql: "SELECT * FROM infer_schema(location = '@data/parquet/int96.parquet')",
+        },
+        SqlTestCase {
+            name: "obfuscate_named_arguments_require_fat_arrow",
+            description: "OBFUSCATE should surface the same '=>' hint when a named argument is written with '='.",
+            setup_sqls: &["CREATE TABLE t1(a String)"],
+            sql: "SELECT * FROM obfuscate(t1, seed = 20)",
+        },
     ];
 
     run_binder_cases("binder_clauses.txt", &cases).await

src/query/sql/tests/it/semantic/binder_clauses.txt

@@ -257,3 +257,17 @@ EvalScalar
             └── limit: NONE
 
 
+=== table_function_named_arguments_require_fat_arrow ===
+description: A table function named argument written with '=' should produce a direct hint to use '=>'.
+sql: SELECT * FROM infer_schema(location = '@data/parquet/int96.parquet')
+status: error
+code: 1065
+message: Table function 'infer_schema' uses `=>` for named arguments; replace `location = ...` with `location => ...`.
+
+=== obfuscate_named_arguments_require_fat_arrow ===
+description: OBFUSCATE should surface the same '=>' hint when a named argument is written with '='.
+sql: SELECT * FROM obfuscate(t1, seed = 20)
+status: error
+code: 1065
+message: Table function 'obfuscate' uses `=>` for named arguments; replace `seed = ...` with `seed => ...`.
+