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/12/31 10:16:49 UTC

[GitHub] [arrow] Dandandan opened a new pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

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


   ## Context
   The context of this PR is that I want to experiment with a different, simple implementation of the hash join implementation which directly can index the build-side array instead of keeping a list of batches. This array could grow beyond 2 ^ 32 billion elements, so would need indexes of type `UInt64` rather than `UInt32`.
   
   ## Implementation
   
   In the PR I just extend the public `take` to take any `IndexType` which implements `ArrowNumericType` and `ToPrimitive`.
   I am not sure about the consideration before to restrict `take` to only `UInt32Array`.


----------------------------------------------------------------
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 commented on pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

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


   Ah, I hadn't yet seen https://github.com/apache/arrow/pull/9061  which appears to fix the clippy errors -- thanks @Dandandan !


----------------------------------------------------------------
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 pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

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


   Rebased


----------------------------------------------------------------
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 commented on pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

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


   The full set of Rust CI tests did not run on this PR :(
   
   Can you please rebase this PR against [apache/master](https://github.com/apache/arrow) to pick up the changes in https://github.com/apache/arrow/pull/9056 so that they do? 
   
   I apologize for the inconvenience. 


----------------------------------------------------------------
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 commented on pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

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


   The clippy failures seem unrelated to your change -- let me check that out...


----------------------------------------------------------------
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] codecov-io edited a comment on pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #9057:
URL: https://github.com/apache/arrow/pull/9057#issuecomment-752993039


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=h1) Report
   > Merging [#9057](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=desc) (571bd65) into [master](https://codecov.io/gh/apache/arrow/commit/4b7cdcb9220b6d94b251aef32c21ef9b4097ecfa?el=desc) (4b7cdcb) will **not change** coverage.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/9057/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #9057   +/-   ##
   =======================================
     Coverage   82.61%   82.61%           
   =======================================
     Files         203      204    +1     
     Lines       50140    50140           
   =======================================
     Hits        41422    41422           
     Misses       8718     8718           
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/compute/kernels/take.rs](https://codecov.io/gh/apache/arrow/pull/9057/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY29tcHV0ZS9rZXJuZWxzL3Rha2UucnM=) | `95.21% <100.00%> (ø)` | |
   | [rust/datafusion/src/physical\_plan/hash\_join.rs](https://codecov.io/gh/apache/arrow/pull/9057/diff?src=pr&el=tree#diff-cnVzdC9kYXRhZnVzaW9uL3NyYy9waHlzaWNhbF9wbGFuL2hhc2hfam9pbi5ycw==) | `89.53% <100.00%> (ø)` | |
   | [rust/arrow/src/csv/writer.rs](https://codecov.io/gh/apache/arrow/pull/9057/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY3N2L3dyaXRlci5ycw==) | `78.82% <0.00%> (-0.56%)` | :arrow_down: |
   | [rust/arrow/src/util/serialization.rs](https://codecov.io/gh/apache/arrow/pull/9057/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvdXRpbC9zZXJpYWxpemF0aW9uLnJz) | `100.00% <0.00%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=footer). Last update [4b7cdcb...571bd65](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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 pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

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


   @jorgecarleitao restarted the CI 😄 


----------------------------------------------------------------
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] jorgecarleitao commented on pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

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


   Clippy missing xD


----------------------------------------------------------------
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 pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

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


   FYI @jorgecarleitao @andygrove this (small) PR is needed to finish https://github.com/apache/arrow/pull/9070 


----------------------------------------------------------------
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 #9057: ARROW-11086: [Rust] Extend take implementation to more index types

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


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


----------------------------------------------------------------
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] codecov-io commented on pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #9057:
URL: https://github.com/apache/arrow/pull/9057#issuecomment-752993039


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=h1) Report
   > Merging [#9057](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=desc) (c0af0d9) into [master](https://codecov.io/gh/apache/arrow/commit/dd5fe7095bb662236e27d3343eb82bc4375f93ef?el=desc) (dd5fe70) will **increase** coverage by `0.00%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/9057/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master    #9057   +/-   ##
   =======================================
     Coverage   82.61%   82.61%           
   =======================================
     Files         202      202           
     Lines       50052    50052           
   =======================================
   + Hits        41350    41351    +1     
   + Misses       8702     8701    -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/compute/kernels/take.rs](https://codecov.io/gh/apache/arrow/pull/9057/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvY29tcHV0ZS9rZXJuZWxzL3Rha2UucnM=) | `95.21% <100.00%> (ø)` | |
   | [rust/datafusion/src/physical\_plan/hash\_join.rs](https://codecov.io/gh/apache/arrow/pull/9057/diff?src=pr&el=tree#diff-cnVzdC9kYXRhZnVzaW9uL3NyYy9waHlzaWNhbF9wbGFuL2hhc2hfam9pbi5ycw==) | `89.94% <100.00%> (ø)` | |
   | [rust/parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow/pull/9057/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9lbmNvZGluZ3MvZW5jb2RpbmcucnM=) | `95.43% <0.00%> (+0.19%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=footer). Last update [dd5fe70...c0af0d9](https://codecov.io/gh/apache/arrow/pull/9057?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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] jorgecarleitao closed pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

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


   


----------------------------------------------------------------
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 edited a comment on pull request #9057: ARROW-11086: [Rust] Extend take implementation to more index types

Posted by GitBox <gi...@apache.org>.
alamb edited a comment on pull request #9057:
URL: https://github.com/apache/arrow/pull/9057#issuecomment-753300901


   The clippy failures in https://github.com/apache/arrow/pull/9057/checks?check_run_id=1630788725  seem unrelated to your change -- let me check that out...


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