You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "onlyjackfrost (via GitHub)" <gi...@apache.org> on 2025/03/13 15:45:03 UTC
[PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
onlyjackfrost opened a new pull request, #15209:
URL: https://github.com/apache/datafusion/pull/15209
## Which issue does this PR close?
- Closes #14433
## What changes are included in this PR?
attach a Diagnostic when dealing with "incompatible type in unary expression" error
## Are these changes tested
Yes, I've tested the PLUS operator with column and non-column operands.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "onlyjackfrost (via GitHub)" <gi...@apache.org>.
onlyjackfrost commented on PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#issuecomment-2724260125
@eliaperantoni,
> Is there any way you can extend it to all unary expressions
Sure. I was thinking about doing the same data type check in other unary operator.
Does it make sense to you?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#issuecomment-2734188614
Thanks again everyone!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "onlyjackfrost (via GitHub)" <gi...@apache.org>.
onlyjackfrost commented on code in PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#discussion_r2001158621
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -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);
+ }
Review Comment:
# from the reply above
> note (without span) = "+ can only be used with numbers, intervals, and timestamps"
> help (without span) = "perhaps you need to cast {expression}"
oops... I might have misunderstood your reply.
I'll remove the condition from here.
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -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);
+ }
Review Comment:
from the reply above
> note (without span) = "+ can only be used with numbers, intervals, and timestamps"
> help (without span) = "perhaps you need to cast {expression}"
oops... I might have misunderstood your reply.
I'll remove the condition from here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "onlyjackfrost (via GitHub)" <gi...@apache.org>.
onlyjackfrost commented on code in PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#discussion_r1995299494
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -45,7 +45,13 @@ 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 spans = operand.spans();
+ let span = if let Some(s) = spans.as_ref() { s.first() } else { None };
+ let diagnostic =
+ Diagnostic::new_error("Unary operator '+' only supports numeric, interval and timestamp types".to_string(), span);
Review Comment:
Sure, it will be better this way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "eliaperantoni (via GitHub)" <gi...@apache.org>.
eliaperantoni commented on PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#issuecomment-2728582201
> @eliaperantoni could I raise another PR for the others unary expressions and keep this PR for the PLUS unary expression?
Yes absolutely 😊
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "eliaperantoni (via GitHub)" <gi...@apache.org>.
eliaperantoni commented on code in PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#discussion_r1995063039
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -45,7 +45,13 @@ 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 spans = operand.spans();
+ let span = if let Some(s) = spans.as_ref() { s.first() } else { None };
Review Comment:
```suggestion
let spans = operand.spans().map(|s| s.first());
```
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -45,7 +45,13 @@ 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 spans = operand.spans();
+ let span = if let Some(s) = spans.as_ref() { s.first() } else { None };
+ let diagnostic =
+ Diagnostic::new_error("Unary operator '+' only supports numeric, interval and timestamp types".to_string(), span);
Review Comment:
Not a big deal but I think this message is a bit long for a `Diagnostic` and it addresses the problem in a very broad way, saying "+ only works with X, Y, and Z" instead of simply "+ cannot be used with W".
Is it possible to make the `Diagnostic` look like this:
- message = "+ cannot be used with {type-name}"
- note (without span) = "+ can only be used with numbers, intervals, and timestamps"
- help (without span) = "perhaps you need to cast {expression}"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "eliaperantoni (via GitHub)" <gi...@apache.org>.
eliaperantoni commented on code in PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#discussion_r1998206440
##########
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*/ ";
Review Comment:
I would remove the span tags `/*whole*/` since they aren't used by the test anyway
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -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);
+ }
Review Comment:
Why do this only when the `span` is `None`? I think it should always be there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "onlyjackfrost (via GitHub)" <gi...@apache.org>.
onlyjackfrost commented on code in PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#discussion_r2001158621
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -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);
+ }
Review Comment:
> note (without span) = "+ can only be used with numbers, intervals, and timestamps"
> help (without span) = "perhaps you need to cast {expression}"
oops... I might have misunderstood your reply.
I'll remove the condition from here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "Weijun-H (via GitHub)" <gi...@apache.org>.
Weijun-H commented on code in PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#discussion_r1994893409
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -45,7 +45,13 @@ 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 spans = operand.spans();
+ let span = if let Some(s) = spans.as_ref() { s.first() } else { None };
+ let diagnostic =
+ Diagnostic::new_error("Unary operator '+' only supports numeric, interval and timestamp types".to_string(), span);
+ e.with_diagnostic(diagnostic)
Review Comment:
Could we add a note or help here, such as `Try using arrow_cast to convert the value to a numeric, interval, or timestamp type`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #15209:
URL: https://github.com/apache/datafusion/pull/15209
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "eliaperantoni (via GitHub)" <gi...@apache.org>.
eliaperantoni commented on code in PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#discussion_r2001166961
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -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);
+ }
Review Comment:
Ahh sorry! Hahah. Yeah I meant to say that the note and the help should have no `Span` attached to them, in my opinion. But they should always be there.
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -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);
+ }
Review Comment:
Ahh sorry! Hahah. Yeah I meant to say that the note and the help should have no `Span` attached to them, in my opinion. But they (the note and the help) should always be there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "eliaperantoni (via GitHub)" <gi...@apache.org>.
eliaperantoni commented on PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#issuecomment-2724283974
@onlyjackfrost
> the same data type check in other unary operator.
I'm not 100% what you mean by that. For `+` you used the existing error that was returned and added a `Diagnostic` to it. As long as other unary operators already have good error coverage, I think adding `Diagnostic`s to them is perfectly good :)
I just hope that all necessary checks already happen in that file, and not "further up" cause that would require more work probably.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "onlyjackfrost (via GitHub)" <gi...@apache.org>.
onlyjackfrost commented on PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#issuecomment-2725191499
@eliaperantoni could I raise another PR for the others unary expressions and keep this PR for the PLUS unary expression?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "onlyjackfrost (via GitHub)" <gi...@apache.org>.
onlyjackfrost commented on PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#issuecomment-2721725989
@eliaperantoni could you help review? :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "Weijun-H (via GitHub)" <gi...@apache.org>.
Weijun-H commented on code in PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#discussion_r1994893409
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -45,7 +45,13 @@ 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 spans = operand.spans();
+ let span = if let Some(s) = spans.as_ref() { s.first() } else { None };
+ let diagnostic =
+ Diagnostic::new_error("Unary operator '+' only supports numeric, interval and timestamp types".to_string(), span);
+ e.with_diagnostic(diagnostic)
Review Comment:
Could we add a note or help here, such as `Try using arrow_cast to convert the value to a numeric, interval, or timestamp type`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "onlyjackfrost (via GitHub)" <gi...@apache.org>.
onlyjackfrost commented on code in PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#discussion_r2001166006
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -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);
+ }
Review Comment:
delivered in https://github.com/apache/datafusion/pull/15209/commits/ef3fca807fb2069c767c92fe3d2a1f9235003159
##########
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*/ ";
Review Comment:
delivered in https://github.com/apache/datafusion/pull/15209/commits/ef3fca807fb2069c767c92fe3d2a1f9235003159
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Attach Diagnostic to "incompatible type in unary expression" error [datafusion]
Posted by "onlyjackfrost (via GitHub)" <gi...@apache.org>.
onlyjackfrost commented on code in PR #15209:
URL: https://github.com/apache/datafusion/pull/15209#discussion_r2001158621
##########
datafusion/sql/src/expr/unary_op.rs:
##########
@@ -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);
+ }
Review Comment:
```
# from the reply above
note (without span) = "+ can only be used with numbers, intervals, and timestamps"
help (without span) = "perhaps you need to cast {expression}"
```
oops... I might have misunderstood your reply.
I'll remove the condition from here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org