You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/01/30 17:12:15 UTC

[GitHub] [arrow] Dandandan opened a new pull request #9374: ARROW-11439: [Rust] Add year support to temporal kernels

Dandandan opened a new pull request #9374:
URL: https://github.com/apache/arrow/pull/9374


   This adds year support to the temporal module. Year support is something needed for some TCPH queries.
   Together with `extract` support
   https://github.com/apache/arrow/pull/9359
   
   we should be able to add `EXTRACT (YEAR FROM dt)` support to DataFusion.
   
   Other changes in the PR:
   * Adding some more tests to `hour`
   * Removing datatype check from inner loop (there is still one more check in `value_as_datetime` and `value_as_time`) but I leave that for a future PR, as well as further performance improvements (e.g. avoiding the `Int32Builder`, avoiding null checks, adding some microbenchmarks etc.).
   * Returning an error message on unsupported datatypes (instead of returning `None`). This is backwards incompatible, but I think this is reasonable.
   
   
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] nevi-me commented on a change in pull request #9374: ARROW-11439: [Rust] Add year support to temporal kernels

Posted by GitBox <gi...@apache.org>.
nevi-me commented on a change in pull request #9374:
URL: https://github.com/apache/arrow/pull/9374#discussion_r567891864



##########
File path: rust/arrow/src/compute/kernels/temporal.rs
##########
@@ -17,34 +17,82 @@
 
 //! Defines temporal kernels for time and date related functions.
 
-use chrono::Timelike;
+use chrono::{Datelike, Timelike};
 
-use crate::array::*;
 use crate::datatypes::*;
 use crate::error::Result;
-
+use crate::{array::*, error::ArrowError};

Review comment:
       nit: rust-analyzer bunches imports in an off way (assuming you're using it). Sometimes they need reordering manually (e.g. grouping `Result` and `ArrowError`)




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #9374: ARROW-11439: [Rust] Add year support to temporal kernels

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9374:
URL: https://github.com/apache/arrow/pull/9374#issuecomment-770245652


   https://issues.apache.org/jira/browse/ARROW-11439


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] Dandandan commented on a change in pull request #9374: ARROW-11439: [Rust] Add year support to temporal kernels

Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #9374:
URL: https://github.com/apache/arrow/pull/9374#discussion_r567931551



##########
File path: rust/arrow/src/compute/kernels/temporal.rs
##########
@@ -17,34 +17,82 @@
 
 //! Defines temporal kernels for time and date related functions.
 
-use chrono::Timelike;
+use chrono::{Datelike, Timelike};
 
-use crate::array::*;
 use crate::datatypes::*;
 use crate::error::Result;
-
+use crate::{array::*, error::ArrowError};

Review comment:
       Good catch. And you are right, I do use rust-analyzer.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [arrow] alamb closed pull request #9374: ARROW-11439: [Rust] Add year support to temporal kernels

Posted by GitBox <gi...@apache.org>.
alamb closed pull request #9374:
URL: https://github.com/apache/arrow/pull/9374


   


----------------------------------------------------------------
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:
users@infra.apache.org