alamb commented on a change in pull request #8076:
URL: https://github.com/apache/arrow/pull/8076#discussion_r480022925
##########
File path: rust/datafusion/src/execution/physical_plan/expressions.rs
##########
@@ -991,48 +991,36 @@ impl fmt::Display for BinaryExpr {
}
}
-// Returns a formatted error about being impossible to coerce types for the
binary operator.
-fn coercion_error<T>(
- lhs_type: &DataType,
- op: &Operator,
- rhs_type: &DataType,
-) -> Result<T> {
+// Returns a formatted error about being impossible to coerce types to a
common type
+fn coercion_error<T>(lhs_type: &DataType, rhs_type: &DataType) -> Result<T> {
Err(ExecutionError::General(
format!(
- "The binary operator '{}' can't evaluate with lhs = '{:?}' and rhs
= '{:?}'",
Review comment:
Can you explain why you removed the mention of which operator (`op`) was
affected in the error message?
There is probably some reason, but on the face of it, this PR seems to make
the errors less specific as now there is no hint to a user what operator is
having the problem - just "somewhere in your expressions I couldn't coerce
types"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
