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/06/22 16:14:13 UTC
[GitHub] [arrow-rs] b41sh opened a new pull request #490: Implement function slice for RecordBatch
b41sh opened a new pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490
# Which issue does this PR close?
<!---
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #460
# Rationale for this change
<!---
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
`slice` can be used to handle part of `RecordBatch`
# What changes are included in this PR?
<!---
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
Implement function `slice` for `RecordBatch`
# Are there any user-facing changes?
<!---
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
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-rs] Dandandan commented on a change in pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
Dandandan commented on a change in pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#discussion_r657724817
##########
File path: arrow/src/record_batch.rs
##########
@@ -426,6 +441,29 @@ mod tests {
assert_eq!(5, record_batch.column(1).data().len());
}
+ #[test]
+ fn create_record_batch_slice() {
Review comment:
It should return an empty slice.
--
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-rs] b41sh commented on a change in pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#discussion_r656393219
##########
File path: arrow/src/record_batch.rs
##########
@@ -244,6 +244,18 @@ impl RecordBatch {
&self.columns[..]
}
+ /// Return a new RecordBatch where each column is sliced
+ /// according to `offset` and `length`
+ pub fn slice(&self, offset: usize, length: usize) -> Result<RecordBatch> {
+ let schema = self.schema();
+ let num_columns = self.num_columns();
+ let new_columns = (0..num_columns)
Review comment:
@jorgecarleitao thanks, I have modified, PTAL
--
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-rs] alamb commented on pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#issuecomment-866737245
Thank you @b41sh !
--
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-rs] codecov-commenter commented on pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#issuecomment-867445169
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#490](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (856d8c4) into [master](https://codecov.io/gh/apache/arrow-rs/commit/4c7d4189e72901a78fb4f4250c11421241dd9e13?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c7d418) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #490 +/- ##
==========================================
+ Coverage 82.65% 82.66% +0.01%
==========================================
Files 165 165
Lines 45524 45543 +19
==========================================
+ Hits 37628 37650 +22
+ Misses 7896 7893 -3
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/record\_batch.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL3JlY29yZF9iYXRjaC5ycw==) | `87.11% <100.00%> (+1.39%)` | :arrow_up: |
| [parquet/src/util/bit\_packing.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvdXRpbC9iaXRfcGFja2luZy5ycw==) | `99.96% <0.00%> (-0.01%)` | :arrow_down: |
| [arrow/src/array/equal/variable\_size.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2VxdWFsL3ZhcmlhYmxlX3NpemUucnM=) | `100.00% <0.00%> (ø)` | |
| [parquet/src/schema/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvc2NoZW1hL3R5cGVzLnJz) | `88.18% <0.00%> (+0.11%)` | :arrow_up: |
| [parquet/src/encodings/rle.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvZW5jb2RpbmdzL3JsZS5ycw==) | `92.96% <0.00%> (+0.24%)` | :arrow_up: |
| [parquet/src/arrow/record\_reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvYXJyb3cvcmVjb3JkX3JlYWRlci5ycw==) | `93.73% <0.00%> (+0.28%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4c7d418...856d8c4](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-rs] Jimexist commented on a change in pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
Jimexist commented on a change in pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#discussion_r656770581
##########
File path: arrow/src/record_batch.rs
##########
@@ -244,6 +244,21 @@ impl RecordBatch {
&self.columns[..]
}
+ /// Return a new RecordBatch where each column is sliced
+ /// according to `offset` and `length`
+ pub fn slice(&self, offset: usize, length: usize) -> Result<RecordBatch> {
Review comment:
can you remove the `Result` wrapping if nothing can fail.
--
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-rs] codecov-commenter edited a comment on pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#issuecomment-867445169
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#490](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9aacab1) into [master](https://codecov.io/gh/apache/arrow-rs/commit/4c7d4189e72901a78fb4f4250c11421241dd9e13?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c7d418) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #490 +/- ##
==========================================
- Coverage 82.65% 82.64% -0.02%
==========================================
Files 165 165
Lines 45524 45703 +179
==========================================
+ Hits 37628 37769 +141
- Misses 7896 7934 +38
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/record\_batch.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL3JlY29yZF9iYXRjaC5ycw==) | `89.25% <100.00%> (+3.53%)` | :arrow_up: |
| [arrow/src/array/transform/boolean.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L3RyYW5zZm9ybS9ib29sZWFuLnJz) | `76.92% <0.00%> (-7.70%)` | :arrow_down: |
| [arrow/src/ffi.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2ZmaS5ycw==) | `78.71% <0.00%> (-4.15%)` | :arrow_down: |
| [arrow/src/compute/kernels/temporal.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy90ZW1wb3JhbC5ycw==) | `88.97% <0.00%> (-0.64%)` | :arrow_down: |
| [parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvZW5jb2RpbmdzL2VuY29kaW5nLnJz) | `94.85% <0.00%> (-0.20%)` | :arrow_down: |
| [arrow/src/array/array\_list.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2FycmF5X2xpc3QucnM=) | `92.82% <0.00%> (ø)` | |
| [arrow/src/compute/kernels/concat.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9jb25jYXQucnM=) | `100.00% <0.00%> (ø)` | |
| [arrow/src/array/equal/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2VxdWFsL21vZC5ycw==) | `93.93% <0.00%> (+0.34%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4c7d418...9aacab1](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-rs] b41sh commented on a change in pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#discussion_r657853819
##########
File path: arrow/src/record_batch.rs
##########
@@ -426,6 +441,29 @@ mod tests {
assert_eq!(5, record_batch.column(1).data().len());
}
+ #[test]
+ fn create_record_batch_slice() {
Review comment:
I have modified, PTAL
@Dandandan @Jimexist
--
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-rs] codecov-commenter edited a comment on pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#issuecomment-867445169
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#490](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (34f7194) into [master](https://codecov.io/gh/apache/arrow-rs/commit/4c7d4189e72901a78fb4f4250c11421241dd9e13?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c7d418) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #490 +/- ##
==========================================
- Coverage 82.65% 82.64% -0.01%
==========================================
Files 165 165
Lines 45524 45703 +179
==========================================
+ Hits 37628 37773 +145
- Misses 7896 7930 +34
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [arrow/src/record\_batch.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL3JlY29yZF9iYXRjaC5ycw==) | `89.25% <100.00%> (+3.53%)` | :arrow_up: |
| [arrow/src/ffi.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2ZmaS5ycw==) | `78.71% <0.00%> (-4.15%)` | :arrow_down: |
| [arrow/src/compute/kernels/temporal.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy90ZW1wb3JhbC5ycw==) | `88.97% <0.00%> (-0.64%)` | :arrow_down: |
| [parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvZW5jb2RpbmdzL2VuY29kaW5nLnJz) | `94.85% <0.00%> (-0.20%)` | :arrow_down: |
| [parquet/src/util/bit\_packing.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvdXRpbC9iaXRfcGFja2luZy5ycw==) | `99.96% <0.00%> (-0.01%)` | :arrow_down: |
| [arrow/src/array/array\_list.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2FycmF5X2xpc3QucnM=) | `92.82% <0.00%> (ø)` | |
| [arrow/src/compute/kernels/concat.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2NvbXB1dGUva2VybmVscy9jb25jYXQucnM=) | `100.00% <0.00%> (ø)` | |
| [arrow/src/array/equal/variable\_size.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2FycmF5L2VxdWFsL3ZhcmlhYmxlX3NpemUucnM=) | `100.00% <0.00%> (ø)` | |
| [parquet/src/schema/types.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvc2NoZW1hL3R5cGVzLnJz) | `88.18% <0.00%> (+0.11%)` | :arrow_up: |
| [parquet/src/encodings/rle.rs](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvZW5jb2RpbmdzL3JsZS5ycw==) | `92.96% <0.00%> (+0.24%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/arrow-rs/pull/490/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4c7d418...34f7194](https://codecov.io/gh/apache/arrow-rs/pull/490?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-rs] b41sh commented on a change in pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#discussion_r656403180
##########
File path: arrow/src/record_batch.rs
##########
@@ -244,6 +244,18 @@ impl RecordBatch {
&self.columns[..]
}
+ /// Return a new RecordBatch where each column is sliced
+ /// according to `offset` and `length`
+ pub fn slice(&self, offset: usize, length: usize) -> Result<RecordBatch> {
+ let schema = self.schema();
+ let num_columns = self.num_columns();
+ let new_columns = (0..num_columns)
Review comment:
@nevi-me Thanks for your suggestion, I made a modification. PTAL
--
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-rs] b41sh commented on pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
b41sh commented on pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#issuecomment-867735578
> Hi @b41sh. Thank you for the contribution.
>
> It appears there are some issues that the CI tests found (clippy): https://github.com/apache/arrow-rs/pull/490/checks?check_run_id=2905850637
>
> Can you please resolve them?
@alamb thanks for your review, I have fixed the Clippy, PTAL
--
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-rs] b41sh commented on a change in pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#discussion_r657722171
##########
File path: arrow/src/record_batch.rs
##########
@@ -426,6 +441,29 @@ mod tests {
assert_eq!(5, record_batch.column(1).data().len());
}
+ #[test]
+ fn create_record_batch_slice() {
Review comment:
@Jimexist I have a question, if an empty RecordBatch calls slice(), the offset and length are zero, should we panic or return an empty RecordBatch slice?
--
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-rs] nevi-me commented on a change in pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
nevi-me commented on a change in pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#discussion_r656390969
##########
File path: arrow/src/record_batch.rs
##########
@@ -244,6 +244,18 @@ impl RecordBatch {
&self.columns[..]
}
+ /// Return a new RecordBatch where each column is sliced
+ /// according to `offset` and `length`
+ pub fn slice(&self, offset: usize, length: usize) -> Result<RecordBatch> {
+ let schema = self.schema();
+ let num_columns = self.num_columns();
+ let new_columns = (0..num_columns)
Review comment:
My local version is
```rust
/// Slice all the arrays in the record batch.
pub fn slice(&self, offset: usize, len: usize) -> Self {
let columns = self
.columns()
.iter()
.map(|array| array.slice(offset, len))
.collect();
Self {
schema: self.schema.clone(),
columns,
}
}
```
Does it make sense to go through `RecordBatch::try_new()`? It incurs some overhead checking that the schema and arrays match, when they already should match
--
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-rs] b41sh commented on a change in pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
b41sh commented on a change in pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#discussion_r657745966
##########
File path: arrow/src/record_batch.rs
##########
@@ -426,6 +441,29 @@ mod tests {
assert_eq!(5, record_batch.column(1).data().len());
}
+ #[test]
+ fn create_record_batch_slice() {
Review comment:
> It should return an empty slice.
ok
--
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-rs] nevi-me merged pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
nevi-me merged pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] Jimexist commented on a change in pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
Jimexist commented on a change in pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#discussion_r656770815
##########
File path: arrow/src/record_batch.rs
##########
@@ -426,6 +441,29 @@ mod tests {
assert_eq!(5, record_batch.column(1).data().len());
}
+ #[test]
+ fn create_record_batch_slice() {
Review comment:
can you test for edge cases, e.g. empty length, over boundary offset, empty columns, empty batch, etc.
--
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-rs] jorgecarleitao commented on a change in pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on a change in pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#discussion_r656382277
##########
File path: arrow/src/record_batch.rs
##########
@@ -244,6 +244,18 @@ impl RecordBatch {
&self.columns[..]
}
+ /// Return a new RecordBatch where each column is sliced
+ /// according to `offset` and `length`
+ pub fn slice(&self, offset: usize, length: usize) -> Result<RecordBatch> {
+ let schema = self.schema();
+ let num_columns = self.num_columns();
+ let new_columns = (0..num_columns)
Review comment:
`self.columns().iter()` ^_^
--
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-rs] alamb commented on pull request #490: Implement function slice for RecordBatch
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #490:
URL: https://github.com/apache/arrow-rs/pull/490#issuecomment-867699277
Hi @b41sh. Thank you for the contribution.
It appears there are some issues that the CI tests found (clippy): https://github.com/apache/arrow-rs/pull/490/checks?check_run_id=2905850637
Can you please resolve them?
--
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