alamb commented on code in PR #8886:
URL: https://github.com/apache/arrow-datafusion/pull/8886#discussion_r1457942652
##########
datafusion/physical-expr/src/datetime_expressions.rs:
##########
@@ -121,58 +291,220 @@ where
}
}
+// given an function that maps a `&str`, `&str` to an arrow native type,
+// returns a `ColumnarValue` where the function is applied to either a
`ArrayRef` or `ScalarValue`
+// depending on the `args`'s variant.
+fn handle_multiple<'a, O, F, S, M>(
+ args: &'a [ColumnarValue],
+ op: F,
+ op2: M,
+ name: &str,
+) -> Result<ColumnarValue>
+where
+ O: ArrowPrimitiveType,
+ S: ScalarType<O::Native>,
+ F: Fn(&'a str, &'a str) -> Result<O::Native>,
+ M: Fn(O::Native) -> O::Native,
+{
+ match &args[0] {
+ ColumnarValue::Array(a) => match a.data_type() {
+ DataType::Utf8 | DataType::LargeUtf8 => {
+ // validate the column types
+ for (pos, arg) in args.iter().enumerate() {
+ match arg {
+ ColumnarValue::Array(arg) => match arg.data_type() {
+ DataType::Utf8 | DataType::LargeUtf8 => {
+ // all good
+ },
+ other => return internal_err!("Unsupported
data type {other:?} for function {name}, arg # {pos}"),
+ },
+ ColumnarValue::Scalar(arg) => { match arg.data_type() {
+ DataType::Utf8 | DataType::LargeUtf8 => {
+ // all good
+ },
+ other => return internal_err!("Unsupported data
type {other:?} for function {name}, arg # {pos}"),
+ }}
+ }
+ }
+
+ Ok(ColumnarValue::Array(Arc::new(
+ strings_to_primitive_function::<i32, O, _, _>(args, op,
op2, name)?,
+ )))
+ }
+ other => {
+ internal_err!("Unsupported data type {other:?} for function
{name}")
+ }
+ },
+ // if the first argument is a scalar utf8 all arguments are expected
to be scalar utf8
+ ColumnarValue::Scalar(scalar) => match scalar {
+ ScalarValue::Utf8(a) | ScalarValue::LargeUtf8(a) => {
+ let mut val: Option<Result<ColumnarValue>> = None;
+ let mut err: Option<DataFusionError> = None;
+
+ match a {
+ Some(a) => {
+ // enumerate all the values finding the first one that
returns an Ok result
+ for (pos, v) in args.iter().enumerate().skip(1) {
+ if let ColumnarValue::Scalar(s) = v {
+ if let ScalarValue::Utf8(x) |
ScalarValue::LargeUtf8(x) =
+ s
+ {
+ if let Some(s) = x {
+ match op(a.as_str(), s.as_str()) {
+ Ok(r) => {
+ val =
Some(Ok(ColumnarValue::Scalar(
+ S::scalar(Some(op2(r))),
+ )));
+ break;
+ }
+ Err(e) => {
+ err = Some(e);
+ }
+ }
+ }
+ } else {
+ return internal_err!("Unsupported data
type {s:?} for function {name}, arg # {pos}");
Review Comment:
I agree it is not required to support the arguments -- I mostly think the
error type should not be "internal error" as that signals a bug in datafusion,
which I don't think this is:
https://github.com/apache/arrow-datafusion/blob/d14f766b5d3c117d5027f032b38837ec4e285b8a/datafusion/common/src/error.rs#L74-L90
> I was thinking about ignoring any argument that are not Utf8/LargeUtf8 as
long as there was at least one arg that was acceptable but I didn't think that
was a good idea in general as it hides what to me is an obvious error.
I agree
--
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]
