theirix commented on code in PR #22244:
URL: https://github.com/apache/datafusion/pull/22244#discussion_r3293278859
##########
datafusion/functions/src/string/concat.rs:
##########
@@ -67,27 +68,19 @@ impl Default for ConcatFunc {
impl ConcatFunc {
pub fn new() -> Self {
- use DataType::*;
Self {
- signature: Signature::variadic(
- vec![Utf8View, Utf8, LargeUtf8, Binary],
- Volatility::Immutable,
- ),
+ // Use `Signature::UserDefined` to allow different argument types.
+ // `Variadic` requires every argument to be coerced to the same
string type,
+ // so the UDF cannot distinguish between binary and string inputs.
+ signature: Signature::user_defined(Volatility::Immutable),
}
}
}
-fn deduce_return_type(arg_types: &[DataType]) -> DataType {
- use DataType::*;
- if arg_types.contains(&Utf8View) {
- Utf8View
- } else if arg_types.contains(&LargeUtf8) {
- LargeUtf8
- } else {
- Utf8
- }
-}
-
+// Logic is matched with pipe operator in the following table.
+// Support only string + string concatenation,
+// or binary + binary concatenation.
+// Mixed string + binary concatenation is rejected,
Review Comment:
> I'll try take a proper look at this, but might take some time as there
seems to be quite a lot happening in this PR.
> cc @neilconway as you worked on the concat builder code previously
Thank you! Also, I kindly ask @kosiew who reviewed the previous PR #20787
> I think this might constitute a breaking change 🤔
The preliminary mixed binary support was introduced recently in #20787 (I
guess it is not released yet - 53.1.0 or so), and now we're removing it to make
it more consistent. I tried to describe the logic and rationale in the PR
description
--
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]
