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 2022/07/08 18:35:47 UTC
[GitHub] [arrow-rs] tustvold opened a new pull request, #2032: Add more tests of #2025
tustvold opened a new pull request, #2032:
URL: https://github.com/apache/arrow-rs/pull/2032
Follow on from #2027, this just tests a few more edge cases
--
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] codecov-commenter commented on pull request #2032: Add more tests of RecordReader Batch Size Edge Cases (#2025)
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2032:
URL: https://github.com/apache/arrow-rs/pull/2032#issuecomment-1185719039
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/2032?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 [#2032](https://codecov.io/gh/apache/arrow-rs/pull/2032?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d2ce4b6) into [master](https://codecov.io/gh/apache/arrow-rs/commit/5a76697eb69e5c21aa60348e32d08069942ffa09?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a76697) will **increase** coverage by `0.13%`.
> The diff coverage is `92.88%`.
> :exclamation: Current head d2ce4b6 differs from pull request most recent head 4c793ea. Consider uploading reports for the commit 4c793ea to get more accurate results
```diff
@@ Coverage Diff @@
## master #2032 +/- ##
==========================================
+ Coverage 83.55% 83.68% +0.13%
==========================================
Files 222 224 +2
Lines 58230 58833 +603
==========================================
+ Hits 48654 49236 +582
- Misses 9576 9597 +21
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/2032?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/array/builder/primitive\_builder.rs](https://codecov.io/gh/apache/arrow-rs/pull/2032/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-YXJyb3cvc3JjL2FycmF5L2J1aWxkZXIvcHJpbWl0aXZlX2J1aWxkZXIucnM=) | `92.57% <ø> (-0.55%)` | :arrow_down: |
| [arrow/src/array/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/2032/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-YXJyb3cvc3JjL2FycmF5L21vZC5ycw==) | `100.00% <ø> (ø)` | |
| [arrow/src/datatypes/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/2032/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-YXJyb3cvc3JjL2RhdGF0eXBlcy9tb2QucnM=) | `99.24% <ø> (ø)` | |
| [parquet/src/arrow/async\_reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/2032/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-cGFycXVldC9zcmMvYXJyb3cvYXN5bmNfcmVhZGVyLnJz) | `0.00% <0.00%> (ø)` | |
| [parquet/src/arrow/record\_reader/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/2032/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-cGFycXVldC9zcmMvYXJyb3cvcmVjb3JkX3JlYWRlci9tb2QucnM=) | `89.17% <ø> (ø)` | |
| [parquet/src/column/page.rs](https://codecov.io/gh/apache/arrow-rs/pull/2032/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-cGFycXVldC9zcmMvY29sdW1uL3BhZ2UucnM=) | `98.68% <ø> (ø)` | |
| [parquet/src/util/test\_common/page\_util.rs](https://codecov.io/gh/apache/arrow-rs/pull/2032/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-cGFycXVldC9zcmMvdXRpbC90ZXN0X2NvbW1vbi9wYWdlX3V0aWwucnM=) | `86.73% <0.00%> (ø)` | |
| [arrow/src/datatypes/datatype.rs](https://codecov.io/gh/apache/arrow-rs/pull/2032/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-YXJyb3cvc3JjL2RhdGF0eXBlcy9kYXRhdHlwZS5ycw==) | `64.41% <30.00%> (-1.27%)` | :arrow_down: |
| [parquet/src/file/metadata.rs](https://codecov.io/gh/apache/arrow-rs/pull/2032/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-cGFycXVldC9zcmMvZmlsZS9tZXRhZGF0YS5ycw==) | `94.70% <66.66%> (-0.69%)` | :arrow_down: |
| [arrow/src/ffi.rs](https://codecov.io/gh/apache/arrow-rs/pull/2032/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==) | `87.52% <72.72%> (+0.34%)` | :arrow_up: |
| ... and [29 more](https://codecov.io/gh/apache/arrow-rs/pull/2032/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/2032?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/2032?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 [5a76697...4c793ea](https://codecov.io/gh/apache/arrow-rs/pull/2032?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.
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] tustvold merged pull request #2032: Add more tests of RecordReader Batch Size Edge Cases (#2025)
Posted by GitBox <gi...@apache.org>.
tustvold merged PR #2032:
URL: https://github.com/apache/arrow-rs/pull/2032
--
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] alamb commented on a diff in pull request #2032: Add more tests of RecordReader Batch Size Edge Cases (#2025)
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2032:
URL: https://github.com/apache/arrow-rs/pull/2032#discussion_r922316988
##########
parquet/src/arrow/arrow_reader.rs:
##########
@@ -1564,9 +1563,25 @@ mod tests {
writer.close().unwrap();
let mut file_reader = ParquetFileArrowReader::try_new(Bytes::from(buf)).unwrap();
- let mut record_reader = file_reader.get_record_reader(8).unwrap();
- assert_eq!(8, record_reader.next().unwrap().unwrap().num_rows());
- assert_eq!(8, record_reader.next().unwrap().unwrap().num_rows());
- assert_eq!(4, record_reader.next().unwrap().unwrap().num_rows());
+ let mut record_reader = file_reader.get_record_reader(batch_size).unwrap();
+ assert_eq!(
+ batch_size,
+ record_reader.next().unwrap().unwrap().num_rows()
+ );
+ assert_eq!(
+ batch_size,
+ record_reader.next().unwrap().unwrap().num_rows()
+ );
+ }
+
+ #[test]
+ fn test_row_group_exact_multiple() {
+ use crate::arrow::record_reader::MIN_BATCH_SIZE;
+ test_row_group_batch(8, 8);
Review Comment:
👍 love it
##########
parquet/src/arrow/arrow_reader.rs:
##########
@@ -1564,9 +1563,25 @@ mod tests {
writer.close().unwrap();
let mut file_reader = ParquetFileArrowReader::try_new(Bytes::from(buf)).unwrap();
- let mut record_reader = file_reader.get_record_reader(8).unwrap();
- assert_eq!(8, record_reader.next().unwrap().unwrap().num_rows());
- assert_eq!(8, record_reader.next().unwrap().unwrap().num_rows());
- assert_eq!(4, record_reader.next().unwrap().unwrap().num_rows());
+ let mut record_reader = file_reader.get_record_reader(batch_size).unwrap();
+ assert_eq!(
+ batch_size,
+ record_reader.next().unwrap().unwrap().num_rows()
+ );
+ assert_eq!(
+ batch_size,
+ record_reader.next().unwrap().unwrap().num_rows()
+ );
+ }
+
+ #[test]
+ fn test_row_group_exact_multiple() {
+ use crate::arrow::record_reader::MIN_BATCH_SIZE;
+ test_row_group_batch(8, 8);
Review Comment:
```suggestion
test_row_group_batch(8, 8);
test_row_group_batch(10, 8);
test_row_group_batch(8, 10);
```
Would it be worth keeping the `10` and `8` in the current test? As well as 8 and 10 for good measure?
--
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] ursabot commented on pull request #2032: Add more tests of RecordReader Batch Size Edge Cases (#2025)
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #2032:
URL: https://github.com/apache/arrow-rs/pull/2032#issuecomment-1185747509
Benchmark runs are scheduled for baseline = cb7e5b0c8437c3f2ce2ae21951331599de4ccc7d and contender = a7181ddbda801ba021398fb264cda394d64bff85. a7181ddbda801ba021398fb264cda394d64bff85 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/0bf5089daa744c89a4bd911cb57f2ebb...d561e647c1c74498a81f0240f14f3ac8/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/5a0b1acc17fb44d8affa912fc2bffdda...9a56ac3f06564c038380a9d273d6ee0c/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/eca457a1a2b54906acd5f189bccca99a...94a8effdc51440088977bb1f3d7321d0/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/ffc5759191904bcdb852c7a6af434312...5c0ad4c48a514ca3aef1afab75914123/)
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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