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 2020/07/29 07:47:31 UTC

[GitHub] [arrow] jhorstmann opened a new pull request #7854: ARROW-9583 Fix offsets in result of arithmetic kernels

jhorstmann opened a new pull request #7854:
URL: https://github.com/apache/arrow/pull/7854


   


----------------------------------------------------------------
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] jhorstmann commented on a change in pull request #7854: ARROW-9583 Fix offsets in result of arithmetic kernels

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



##########
File path: rust/arrow/src/compute/kernels/boolean.rs
##########
@@ -53,13 +53,18 @@ where
         right_data.null_bitmap(),
         |a, b| a & b,
     )?;
-    let values = op(&left_data.buffers()[0], &right_data.buffers()[0])?;
+    let left_buffer = &left_data.buffers()[0];
+    let right_buffer = &right_data.buffers()[0];
+    let values = op(
+        &left_buffer.slice(left_data.offset() / 8),

Review comment:
       This part does not work yet in all cases since we also need to take the length of the array data into account, which might be less than the buffer length.

##########
File path: rust/arrow/src/compute/kernels/boolean.rs
##########
@@ -53,13 +53,18 @@ where
         right_data.null_bitmap(),
         |a, b| a & b,
     )?;
-    let values = op(&left_data.buffers()[0], &right_data.buffers()[0])?;
+    let left_buffer = &left_data.buffers()[0];
+    let right_buffer = &right_data.buffers()[0];
+    let values = op(
+        &left_buffer.slice(left_data.offset() / 8),

Review comment:
       This part does not work yet in all cases since we also need to take the length of the array data into account, which might be less than the buffer length. See testcase `test_bool_array_and_sliced_same_offset_mod8` 




----------------------------------------------------------------
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] jhorstmann commented on pull request #7854: ARROW-9583: [Rust] Fix offsets in result of arithmetic kernels

Posted by GitBox <gi...@apache.org>.
jhorstmann commented on pull request #7854:
URL: https://github.com/apache/arrow/pull/7854#issuecomment-683832637


   @andygrove looks like your commit worked and ci just needed a few minutes to get started


----------------------------------------------------------------
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] jhorstmann commented on pull request #7854: ARROW-9583: [Rust] Fix offsets in result of arithmetic kernels

Posted by GitBox <gi...@apache.org>.
jhorstmann commented on pull request #7854:
URL: https://github.com/apache/arrow/pull/7854#issuecomment-670407634


   @paddyhoran can you take a look at this PR?


----------------------------------------------------------------
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] andygrove commented on pull request #7854: ARROW-9583: [Rust] Fix offsets in result of arithmetic kernels

Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7854:
URL: https://github.com/apache/arrow/pull/7854#issuecomment-684155565


   @paddyhoran @nevi-me If you have the time, would you mind merging this one? I've run into an issue with my Python setup again that won't let me merge PRs if the contributor's name has non-ASCII characters and I don't remember how to resolve that.


----------------------------------------------------------------
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] andygrove commented on pull request #7854: ARROW-9583: [Rust] Fix offsets in result of arithmetic kernels

Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #7854:
URL: https://github.com/apache/arrow/pull/7854#issuecomment-683812033


   @jhorstmann I was trying to trigger CI with an empty commit but it didn't work for some reason. Maybe you could try rebasing or pushing a commit?


----------------------------------------------------------------
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] paddyhoran commented on pull request #7854: ARROW-9583: [Rust] Fix offsets in result of arithmetic kernels

Posted by GitBox <gi...@apache.org>.
paddyhoran commented on pull request #7854:
URL: https://github.com/apache/arrow/pull/7854#issuecomment-670918619


   > @paddyhoran can you take a look at this PR?
   
   Yep, will do.  Apologies, I was on vacation last week.


----------------------------------------------------------------
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] andygrove closed pull request #7854: ARROW-9583: [Rust] Fix offsets in result of arithmetic kernels

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


   


----------------------------------------------------------------
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] jhorstmann commented on pull request #7854: ARROW-9583: [Rust] Fix offsets in result of arithmetic kernels

Posted by GitBox <gi...@apache.org>.
jhorstmann commented on pull request #7854:
URL: https://github.com/apache/arrow/pull/7854#issuecomment-672940720


   Thanks @paddyhoran, I will then look into removing the offsets requirements and into the simd kernels next. I think the `chunked_exact`/`remainder` pattern would work quite well for all the simd kernels.


----------------------------------------------------------------
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 #7854: ARROW-9583 Fix offsets in result of arithmetic kernels

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


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


----------------------------------------------------------------
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