alamb commented on code in PR #8357:
URL: https://github.com/apache/arrow-datafusion/pull/8357#discussion_r1411297026
##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -331,26 +331,27 @@ fn string_temporal_coercion(
rhs_type: &DataType,
) -> Option<DataType> {
use arrow::datatypes::DataType::*;
- match (lhs_type, rhs_type) {
- (Utf8, Date32) | (Date32, Utf8) => Some(Date32),
- (Utf8, Date64) | (Date64, Utf8) => Some(Date64),
- (Utf8, Time32(unit)) | (Time32(unit), Utf8) => {
- match is_time_with_valid_unit(Time32(unit.clone())) {
- false => None,
- true => Some(Time32(unit.clone())),
- }
- }
- (Utf8, Time64(unit)) | (Time64(unit), Utf8) => {
- match is_time_with_valid_unit(Time64(unit.clone())) {
- false => None,
- true => Some(Time64(unit.clone())),
- }
- }
- (Timestamp(_, tz), Utf8) | (Utf8, Timestamp(_, tz)) => {
- Some(Timestamp(TimeUnit::Nanosecond, tz.clone()))
+
+ fn match_rule(l: &DataType, r: &DataType) -> Option<DataType> {
+ match (l, r) {
+ // Coerce Utf8/LargeUtf8 to Date32/Date64/Time32/Time64/Timestamp
+ (Utf8, temporal) | (LargeUtf8, temporal) => match temporal {
+ Date32 | Date64 => Some(temporal.clone()),
+ Time32(_) | Time64(_) => {
+ if is_time_with_valid_unit(temporal.to_owned()) {
+ Some(temporal.to_owned())
+ } else {
+ None
+ }
+ }
+ Timestamp(_, tz) => Some(Timestamp(TimeUnit::Nanosecond,
tz.clone())),
+ _ => None,
+ },
+ _ => None,
}
- _ => None,
}
+
+ match_rule(lhs_type, rhs_type).or_else(|| match_rule(rhs_type, lhs_type))
Review Comment:
This is a neat construction 👍
##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -331,26 +331,27 @@ fn string_temporal_coercion(
rhs_type: &DataType,
) -> Option<DataType> {
use arrow::datatypes::DataType::*;
- match (lhs_type, rhs_type) {
- (Utf8, Date32) | (Date32, Utf8) => Some(Date32),
- (Utf8, Date64) | (Date64, Utf8) => Some(Date64),
- (Utf8, Time32(unit)) | (Time32(unit), Utf8) => {
- match is_time_with_valid_unit(Time32(unit.clone())) {
- false => None,
- true => Some(Time32(unit.clone())),
- }
- }
- (Utf8, Time64(unit)) | (Time64(unit), Utf8) => {
- match is_time_with_valid_unit(Time64(unit.clone())) {
- false => None,
- true => Some(Time64(unit.clone())),
- }
- }
- (Timestamp(_, tz), Utf8) | (Utf8, Timestamp(_, tz)) => {
- Some(Timestamp(TimeUnit::Nanosecond, tz.clone()))
+
+ fn match_rule(l: &DataType, r: &DataType) -> Option<DataType> {
+ match (l, r) {
+ // Coerce Utf8/LargeUtf8 to Date32/Date64/Time32/Time64/Timestamp
+ (Utf8, temporal) | (LargeUtf8, temporal) => match temporal {
+ Date32 | Date64 => Some(temporal.clone()),
+ Time32(_) | Time64(_) => {
+ if is_time_with_valid_unit(temporal.to_owned()) {
+ Some(temporal.to_owned())
+ } else {
+ None
+ }
+ }
+ Timestamp(_, tz) => Some(Timestamp(TimeUnit::Nanosecond,
tz.clone())),
+ _ => None,
+ },
+ _ => None,
}
- _ => None,
}
+
+ match_rule(lhs_type, rhs_type).or_else(|| match_rule(rhs_type, lhs_type))
Review Comment:
This is a neat construction 👍
--
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]
