alamb commented on code in PR #7355:
URL: https://github.com/apache/arrow-datafusion/pull/7355#discussion_r1300555480
##########
datafusion/expr/src/built_in_function.rs:
##########
@@ -737,8 +732,7 @@ impl BuiltinScalarFunction {
Utf8 => List(Arc::new(Field::new("item", Utf8, true))),
Null => Null,
_ => {
- // this error is internal as `data_types` should have
captured this.
- return internal_err!(
+ return plan_err!(
Review Comment:
it is true that this is currently caught by higher up type checks, but I
think it is better to throw a plan error here to tell users about the problem
if something changes rather than throw a confusing internal error
Also given that this code is often copy/pasted it probably should default to
returning a nice error message rather than an opaque one
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
