kosiew commented on code in PR #24802:
URL: https://github.com/apache/datafusion/pull/24802#discussion_r3912964652
##########
datafusion/functions/src/unicode/find_in_set.rs:
##########
@@ -110,18 +110,26 @@ impl ScalarUDFImpl for FindInSetFunc {
| ScalarValue::LargeUtf8(str_list),
),
) => {
- let res = match (string, str_list) {
- (Some(string), Some(str_list)) => {
- let position = str_list
+ let position = match (string, str_list) {
+ (Some(string), Some(str_list)) => Some(
+ str_list
.split(',')
.position(|s| s == string)
- .map_or(0, |idx| idx + 1);
-
- Some(position as i32)
- }
+ .map_or(0, |idx| idx + 1),
+ ),
_ => None,
};
- Ok(ColumnarValue::Scalar(ScalarValue::from(res)))
+ // Int64 when the argument is LargeUtf8, matching what
`return_type` promised
Review Comment:
Small suggestion: since this branch selects the result width from
`return_field` rather than directly from the argument, could we phrase this in
terms of the planned return type? Something like `Match the type promised by
return_type` would stay accurate if the signature or coercion behavior changes
in the future. No behavior change needed.
--
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]
