-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ARROW-10816: [Rust][DF] Operations with Intervals #9434
Conversation
@ovr cool :) could you make a new JIRA story for this PR? |
@Dandandan I dont think that I should create a new jira issue, because I didnt implement calculation between timestamp and interval. Btw: There is a problem, DF executes There are two different ideas how to solve it:
|
d4608a9
to
356c300
Compare
|
||
make_type!( | ||
IntervalYearMonthType, | ||
i32, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this comment has nothing to do with changes from this PR; this is something already in master.
fwiw, I do not think this is correct and I opened a issue for this. IMO this should be [i32;2]
or something like that.
As it stands, we will be trying to read an i32
offseted by 4 bytes from an i64
, which is asking us to write masks and other things to get the values.
So, I think that we should hold this PR until we have the interval implementation correct, or I think that this may end up implementing operations under the assumption of a different physical representation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jorgecarleitao You are right, it's will be better.
@alamb @andygrove @nevi-me @jorgecarleitao
What is better to use? Thanks |
I think the "casts" in this case are very cheap -- namely no data is actually copied I am thinking for example calls So I am not sure if there is a major problem or not to have a cast if the cast is effectively doing no real work |
The Apache Arrow Rust community is moving the Rust implementation into its own dedicated github repositories arrow-rs and arrow-datafusion. It is likely we will not merge this PR into this repository Please see the mailing-list thread for more details We expect the process to take a few days and will follow up with a migration plan for the in-flight PRs. |
#10096 has removed the arrow implementation from this repository (it now resides in https://github.com/apache/arrow-rs and https://github.com/apache/arrow-datafusion) in the hopes of streamlining the development process Please re-target this PR (let us know if you need help doing so) to one/both of the new repositories. Thank you for understanding and helping to make arrow-rs and datafusion better |
It's draft, only started to work on it. DRAFT is needed to indicate that I started to work on it.