Copilot commented on code in PR #24365:
URL: https://github.com/apache/datafusion/pull/24365#discussion_r3784255828
##########
datafusion/functions-nested/src/replace.rs:
##########
@@ -367,6 +397,24 @@ impl ScalarUDFImpl for ArrayReplaceAll {
}
}
+/// Return field shared by `array_replace`, `array_replace_n` and
+/// `array_replace_all`: the input list type, except that its inner field is
+/// nullable whenever the replacement element may be null.
+fn replace_return_field(name: &str, arg_fields: &[FieldRef]) ->
Result<FieldRef> {
+ // `array` is at index 0 and `to` at index 2 for all three functions.
+ // `from` never contributes values to the output, so `to` is the only
+ // argument besides `array` that can affect the output's type.
+ let [array_field, _from_field, to_field, ..] = arg_fields else {
+ return exec_err!(
+ "{name} expects at least 3 arguments, got {}",
+ arg_fields.len()
+ );
+ };
Review Comment:
`replace_return_field` is used from `return_field_from_args` (planning-time
type inference). If the argument-count invariant is violated here, it indicates
an internal bug rather than an execution-time error, so returning `exec_err!`
misclassifies the failure as an execution error. Prefer `internal_err!` (or
`plan_err!`) for correct error categorization.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
