alamb commented on a change in pull request #9359:
URL: https://github.com/apache/arrow/pull/9359#discussion_r579652526
##########
File path: rust/datafusion/src/physical_plan/type_coercion.rs
##########
@@ -68,6 +68,29 @@ pub fn data_types(
current_types: &[DataType],
signature: &Signature,
) -> Result<Vec<DataType>> {
+ let valid_types = get_valid_types(signature, current_types)?;
+
+ if valid_types.contains(¤t_types.to_owned()) {
Review comment:
Why can't this be `¤t_types` (aka why does it need a call to
`to_owned` just to immediately borrow from it?)
##########
File path: rust/datafusion/src/physical_plan/functions.rs
##########
@@ -71,6 +71,8 @@ pub enum Signature {
Exact(Vec<DataType>),
/// fixed number of arguments of arbitrary types
Any(usize),
+ /// One of a list of signatures
+ OneOf(Vec<Signature>),
Review comment:
FYI @seddonm1 I am not sure how this affects your string functions /
other postgres function plans
##########
File path: rust/datafusion/tests/sql.rs
##########
@@ -2134,6 +2134,24 @@ async fn crypto_expressions() -> Result<()> {
Ok(())
}
+#[tokio::test]
+async fn extract_date_part() -> Result<()> {
+ let mut ctx = ExecutionContext::new();
+ let sql = "SELECT
Review comment:
👍
##########
File path: rust/datafusion/src/physical_plan/datetime_expressions.rs
##########
@@ -344,6 +347,98 @@ pub fn date_trunc(args: &[ColumnarValue]) ->
Result<ColumnarValue> {
})
}
+macro_rules! extract_date_part {
+ ($ARRAY: expr, $FN:expr) => {
+ match $ARRAY.data_type() {
+ DataType::Date32 => {
+ let array =
$ARRAY.as_any().downcast_ref::<Date32Array>().unwrap();
+ Ok($FN(array)?)
+ }
+ DataType::Date64 => {
+ let array =
$ARRAY.as_any().downcast_ref::<Date64Array>().unwrap();
+ Ok($FN(array)?)
+ }
+ DataType::Timestamp(time_unit, None) => match time_unit {
+ TimeUnit::Second => {
+ let array = $ARRAY
+ .as_any()
+ .downcast_ref::<TimestampSecondArray>()
+ .unwrap();
+ Ok($FN(array)?)
+ }
+ TimeUnit::Millisecond => {
+ let array = $ARRAY
+ .as_any()
+ .downcast_ref::<TimestampMillisecondArray>()
+ .unwrap();
+ Ok($FN(array)?)
+ }
+ TimeUnit::Microsecond => {
+ let array = $ARRAY
+ .as_any()
+ .downcast_ref::<TimestampMicrosecondArray>()
+ .unwrap();
+ Ok($FN(array)?)
+ }
+ TimeUnit::Nanosecond => {
+ let array = $ARRAY
+ .as_any()
+ .downcast_ref::<TimestampNanosecondArray>()
+ .unwrap();
+ Ok($FN(array)?)
+ }
+ },
+ datatype => Err(DataFusionError::Internal(format!(
+ "Extract does not support datatype {:?}",
+ datatype
+ ))),
+ }
+ };
+}
+
+/// DATE_PART SQL function
+pub fn date_part(args: &[ColumnarValue]) -> Result<ColumnarValue> {
+ if args.len() != 2 {
+ return Err(DataFusionError::Execution(
+ "Expected two arguments in DATE_PART".to_string(),
+ ));
+ }
+ let (date_part, array) = (&args[0], &args[1]);
+
+ let date_part = if let ColumnarValue::Scalar(ScalarValue::Utf8(Some(v))) =
date_part {
+ v
+ } else {
+ return Err(DataFusionError::Execution(
+ "First argument of `DATE_PART` must be non-null scalar
Utf8".to_string(),
+ ));
+ };
+
+ let is_scalar = matches!(array, ColumnarValue::Scalar(_));
+
+ let array = match array {
Review comment:
I assume the longer term plan will be to handle the `Scalar` case more
efficiently. This (converting to an array) is fine for now I think
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
