From 16a181b9569f698ff53680bf8bd98f3d3e235884 Mon Sep 17 00:00:00 2001
From: onlyjackfrost <onlyjackfrost@gmail.com>
Date: Fri, 14 Mar 2025 23:42:18 +0800
Subject: [PATCH 1/2] chore: attach diagnostic to unary_op PLUS

---
 datafusion/sql/src/expr/unary_op.rs      | 15 ++++++++++--
 datafusion/sql/tests/cases/diagnostic.rs | 30 ++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/datafusion/sql/src/expr/unary_op.rs b/datafusion/sql/src/expr/unary_op.rs
index a4096ec2355b..92d49c411671 100644
--- a/datafusion/sql/src/expr/unary_op.rs
+++ b/datafusion/sql/src/expr/unary_op.rs
@@ -16,7 +16,7 @@
 // under the License.
 
 use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
-use datafusion_common::{not_impl_err, plan_err, DFSchema, Result};
+use datafusion_common::{not_impl_err, plan_err, DFSchema, Diagnostic, Result};
 use datafusion_expr::{
     type_coercion::{is_interval, is_timestamp},
     Expr, ExprSchemable,
@@ -45,7 +45,18 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
                 {
                     Ok(operand)
                 } else {
-                    plan_err!("Unary operator '+' only supports numeric, interval and timestamp types")
+                    plan_err!("Unary operator '+' only supports numeric, interval and timestamp types").map_err(|e| {
+                        let span = operand.spans().and_then(|s| s.first());
+                        let mut diagnostic = Diagnostic::new_error(
+                            format!("+ cannot be used with {data_type}"), 
+                            span
+                        );
+                        if span.is_none() {
+                            diagnostic.add_note("+ can only be used with numbers, intervals, and timestamps", None);
+                            diagnostic.add_help(format!("perhaps you need to cast {operand}"), None);
+                        }
+                        e.with_diagnostic(diagnostic)
+                    })
                 }
             }
             UnaryOperator::Minus => {
diff --git a/datafusion/sql/tests/cases/diagnostic.rs b/datafusion/sql/tests/cases/diagnostic.rs
index 5481f046e70a..b65f96cf69c8 100644
--- a/datafusion/sql/tests/cases/diagnostic.rs
+++ b/datafusion/sql/tests/cases/diagnostic.rs
@@ -351,6 +351,36 @@ fn test_in_subquery_multiple_columns() -> Result<(), Box<dyn std::error::Error>>
             .collect::<Vec<_>>(),
         vec!["Select only one column in the subquery"]
     );
+    Ok(())
+}
+
+#[test]
+fn test_unary_op_plus_with_column() -> Result<()> {
+    // Test with a direct query that references a column with an incompatible type
+    let query = "SELECT +/*whole*/first_name/*whole*/ FROM person";
+    let spans = get_spans(query);
+    let diag = do_query(query);
+    assert_eq!(diag.message, "+ cannot be used with Utf8");
+    assert_eq!(diag.span, Some(spans["whole"]));
+    Ok(())
+}
 
+#[test]
+fn test_unary_op_plus_with_non_column() -> Result<()> {
+    // create a table with a column of type varchar
+    let query = "SELECT /*whole*/+'a'/*whole*/ ";
+    let diag = do_query(query);
+    assert_eq!(diag.message, "+ cannot be used with Utf8");
+    assert_eq!(
+        diag.notes[0].message,
+        "+ can only be used with numbers, intervals, and timestamps"
+    );
+    assert_eq!(diag.notes[0].span, None);
+    assert_eq!(
+        diag.helps[0].message,
+        "perhaps you need to cast Utf8(\"a\")"
+    );
+    assert_eq!(diag.helps[0].span, None);
+    assert_eq!(diag.span, None);
     Ok(())
 }

From ef3fca807fb2069c767c92fe3d2a1f9235003159 Mon Sep 17 00:00:00 2001
From: onlyjackfrost <onlyjackfrost@gmail.com>
Date: Tue, 18 Mar 2025 22:16:38 +0800
Subject: [PATCH 2/2] remove condition and update test case

---
 datafusion/sql/src/expr/unary_op.rs      |  6 ++----
 datafusion/sql/tests/cases/diagnostic.rs | 10 +++++++++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/datafusion/sql/src/expr/unary_op.rs b/datafusion/sql/src/expr/unary_op.rs
index 92d49c411671..a4a973a74aa2 100644
--- a/datafusion/sql/src/expr/unary_op.rs
+++ b/datafusion/sql/src/expr/unary_op.rs
@@ -51,10 +51,8 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
                             format!("+ cannot be used with {data_type}"), 
                             span
                         );
-                        if span.is_none() {
-                            diagnostic.add_note("+ can only be used with numbers, intervals, and timestamps", None);
-                            diagnostic.add_help(format!("perhaps you need to cast {operand}"), None);
-                        }
+                        diagnostic.add_note("+ can only be used with numbers, intervals, and timestamps", None);
+                        diagnostic.add_help(format!("perhaps you need to cast {operand}"), None);
                         e.with_diagnostic(diagnostic)
                     })
                 }
diff --git a/datafusion/sql/tests/cases/diagnostic.rs b/datafusion/sql/tests/cases/diagnostic.rs
index b65f96cf69c8..db3aad146711 100644
--- a/datafusion/sql/tests/cases/diagnostic.rs
+++ b/datafusion/sql/tests/cases/diagnostic.rs
@@ -362,13 +362,21 @@ fn test_unary_op_plus_with_column() -> Result<()> {
     let diag = do_query(query);
     assert_eq!(diag.message, "+ cannot be used with Utf8");
     assert_eq!(diag.span, Some(spans["whole"]));
+    assert_eq!(
+        diag.notes[0].message,
+        "+ can only be used with numbers, intervals, and timestamps"
+    );
+    assert_eq!(
+        diag.helps[0].message,
+        "perhaps you need to cast person.first_name"
+    );
     Ok(())
 }
 
 #[test]
 fn test_unary_op_plus_with_non_column() -> Result<()> {
     // create a table with a column of type varchar
-    let query = "SELECT /*whole*/+'a'/*whole*/ ";
+    let query = "SELECT +'a'";
     let diag = do_query(query);
     assert_eq!(diag.message, "+ cannot be used with Utf8");
     assert_eq!(