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/15 17:30:53 UTC
[GitHub] [arrow-rs] viirya opened a new pull request, #2083: Add IPC truncation test case for StructArray
viirya opened a new pull request, #2083:
URL: https://github.com/apache/arrow-rs/pull/2083
# 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 #2082.
Related to #2080.
# 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.
-->
# 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.
-->
# 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.
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] viirya commented on a diff in pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#discussion_r922482702
##########
arrow/src/ipc/writer.rs:
##########
@@ -1784,4 +1784,52 @@ mod tests {
assert_eq!(record_batch_slice, deserialized_batch);
}
+
+ #[test]
+ fn truncate_ipc_struct_array() {
+ fn create_batch() -> RecordBatch {
+ let strings: StringArray = [Some("foo"), None, Some("bar"), Some("baz")]
+ .into_iter()
+ .collect();
+ let ints: Int32Array =
+ [Some(0), Some(2), None, Some(1)].into_iter().collect();
+
+ let struct_array = StructArray::from(vec![
+ (
+ Field::new("s", DataType::Utf8, true),
+ Arc::new(strings) as ArrayRef,
+ ),
+ (
+ Field::new("c", DataType::Int32, false),
+ Arc::new(ints) as ArrayRef,
+ ),
+ ]);
+
+ let schema = Schema::new(vec![Field::new(
+ "struct_array",
+ struct_array.data_type().clone(),
+ true,
+ )]);
+
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(struct_array)]).unwrap()
+ }
+
+ let record_batch = create_batch();
+ let record_batch_slice = record_batch.slice(1, 2);
Review Comment:
I see. You are basically concerned about "This makes it unclear if an offset should or should not be applied to the child_array". As the updated offset is applied on both parent array and child arrays, it is somehow obscure that whether we should take parent's offset when working with child arrays.
--
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 commented on a diff in pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#discussion_r922407674
##########
arrow/src/ipc/writer.rs:
##########
@@ -1784,4 +1784,52 @@ mod tests {
assert_eq!(record_batch_slice, deserialized_batch);
}
+
+ #[test]
+ fn truncate_ipc_struct_array() {
+ fn create_batch() -> RecordBatch {
+ let strings: StringArray = [Some("foo"), None, Some("bar"), Some("baz")]
+ .into_iter()
+ .collect();
+ let ints: Int32Array =
+ [Some(0), Some(2), None, Some(1)].into_iter().collect();
+
+ let struct_array = StructArray::from(vec![
+ (
+ Field::new("s", DataType::Utf8, true),
+ Arc::new(strings) as ArrayRef,
+ ),
+ (
+ Field::new("c", DataType::Int32, false),
+ Arc::new(ints) as ArrayRef,
+ ),
+ ]);
+
+ let schema = Schema::new(vec![Field::new(
+ "struct_array",
+ struct_array.data_type().clone(),
+ true,
+ )]);
+
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(struct_array)]).unwrap()
+ }
+
+ let record_batch = create_batch();
+ let record_batch_slice = record_batch.slice(1, 2);
Review Comment:
There is currently an odd hack in Array::slice for StructArray that pushes down the offset to the child arrays. You will need to manually construct the ArrayData for the StructArray in order to properly test the case of a StructArray with a non-zero offset.
--
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 #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
tustvold merged PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083
--
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 #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#issuecomment-1185769391
# [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/2083?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 [#2083](https://codecov.io/gh/apache/arrow-rs/pull/2083?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6e4e872) into [master](https://codecov.io/gh/apache/arrow-rs/commit/cb7e5b0c8437c3f2ce2ae21951331599de4ccc7d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cb7e5b0) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 6e4e872 differs from pull request most recent head fbd6360. Consider uploading reports for the commit fbd6360 to get more accurate results
```diff
@@ Coverage Diff @@
## master #2083 +/- ##
=======================================
Coverage 83.68% 83.69%
=======================================
Files 224 224
Lines 58826 58855 +29
=======================================
+ Hits 49227 49256 +29
Misses 9599 9599
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/2083?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [parquet/src/arrow/record\_reader/mod.rs](https://codecov.io/gh/apache/arrow-rs/pull/2083/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% <ø> (ø)` | |
| [arrow/src/ipc/writer.rs](https://codecov.io/gh/apache/arrow-rs/pull/2083/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-YXJyb3cvc3JjL2lwYy93cml0ZXIucnM=) | `85.46% <100.00%> (+0.41%)` | :arrow_up: |
| [parquet/src/arrow/arrow\_reader.rs](https://codecov.io/gh/apache/arrow-rs/pull/2083/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-cGFycXVldC9zcmMvYXJyb3cvYXJyb3dfcmVhZGVyLnJz) | `92.76% <100.00%> (+0.09%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/2083?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/2083?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 [cb7e5b0...fbd6360](https://codecov.io/gh/apache/arrow-rs/pull/2083?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 commented on a diff in pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#discussion_r922417705
##########
arrow/src/ipc/writer.rs:
##########
@@ -1784,4 +1784,52 @@ mod tests {
assert_eq!(record_batch_slice, deserialized_batch);
}
+
+ #[test]
+ fn truncate_ipc_struct_array() {
+ fn create_batch() -> RecordBatch {
+ let strings: StringArray = [Some("foo"), None, Some("bar"), Some("baz")]
+ .into_iter()
+ .collect();
+ let ints: Int32Array =
+ [Some(0), Some(2), None, Some(1)].into_iter().collect();
+
+ let struct_array = StructArray::from(vec![
+ (
+ Field::new("s", DataType::Utf8, true),
+ Arc::new(strings) as ArrayRef,
+ ),
+ (
+ Field::new("c", DataType::Int32, false),
+ Arc::new(ints) as ArrayRef,
+ ),
+ ]);
+
+ let schema = Schema::new(vec![Field::new(
+ "struct_array",
+ struct_array.data_type().clone(),
+ true,
+ )]);
+
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(struct_array)]).unwrap()
+ }
+
+ let record_batch = create_batch();
+ let record_batch_slice = record_batch.slice(1, 2);
Review Comment:
Created https://github.com/apache/arrow-rs/pull/2085
--
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 commented on a diff in pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#discussion_r922487351
##########
arrow/src/ipc/writer.rs:
##########
@@ -1784,4 +1784,52 @@ mod tests {
assert_eq!(record_batch_slice, deserialized_batch);
}
+
+ #[test]
+ fn truncate_ipc_struct_array() {
+ fn create_batch() -> RecordBatch {
+ let strings: StringArray = [Some("foo"), None, Some("bar"), Some("baz")]
+ .into_iter()
+ .collect();
+ let ints: Int32Array =
+ [Some(0), Some(2), None, Some(1)].into_iter().collect();
+
+ let struct_array = StructArray::from(vec![
+ (
+ Field::new("s", DataType::Utf8, true),
+ Arc::new(strings) as ArrayRef,
+ ),
+ (
+ Field::new("c", DataType::Int32, false),
+ Arc::new(ints) as ArrayRef,
+ ),
+ ]);
+
+ let schema = Schema::new(vec![Field::new(
+ "struct_array",
+ struct_array.data_type().clone(),
+ true,
+ )]);
+
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(struct_array)]).unwrap()
+ }
+
+ let record_batch = create_batch();
+ let record_batch_slice = record_batch.slice(1, 2);
Review Comment:
Precisely 👍
--
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] viirya commented on a diff in pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
viirya commented on code in PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#discussion_r922422098
##########
arrow/src/ipc/writer.rs:
##########
@@ -1784,4 +1784,52 @@ mod tests {
assert_eq!(record_batch_slice, deserialized_batch);
}
+
+ #[test]
+ fn truncate_ipc_struct_array() {
+ fn create_batch() -> RecordBatch {
+ let strings: StringArray = [Some("foo"), None, Some("bar"), Some("baz")]
+ .into_iter()
+ .collect();
+ let ints: Int32Array =
+ [Some(0), Some(2), None, Some(1)].into_iter().collect();
+
+ let struct_array = StructArray::from(vec![
+ (
+ Field::new("s", DataType::Utf8, true),
+ Arc::new(strings) as ArrayRef,
+ ),
+ (
+ Field::new("c", DataType::Int32, false),
+ Arc::new(ints) as ArrayRef,
+ ),
+ ]);
+
+ let schema = Schema::new(vec![Field::new(
+ "struct_array",
+ struct_array.data_type().clone(),
+ true,
+ )]);
+
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(struct_array)]).unwrap()
+ }
+
+ let record_batch = create_batch();
+ let record_batch_slice = record_batch.slice(1, 2);
Review Comment:
> ArrayData::Slice contains a special case for StructArray where it recurses the offset into its children. However, it preserves the offset on the parent ArrayData, in order for the validity buffer to work correctly.
I do see there is an special case, but not understand "it preserves the offset on the parent ArrayData".
```rust
let new_offset = self.offset + offset;
let new_data = ArrayData {
...
offset: new_offset,
...
};
```
It updates new ArrayData with new offset, no?
--
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 commented on a diff in pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#discussion_r922424177
##########
arrow/src/ipc/writer.rs:
##########
@@ -1784,4 +1784,52 @@ mod tests {
assert_eq!(record_batch_slice, deserialized_batch);
}
+
+ #[test]
+ fn truncate_ipc_struct_array() {
+ fn create_batch() -> RecordBatch {
+ let strings: StringArray = [Some("foo"), None, Some("bar"), Some("baz")]
+ .into_iter()
+ .collect();
+ let ints: Int32Array =
+ [Some(0), Some(2), None, Some(1)].into_iter().collect();
+
+ let struct_array = StructArray::from(vec![
+ (
+ Field::new("s", DataType::Utf8, true),
+ Arc::new(strings) as ArrayRef,
+ ),
+ (
+ Field::new("c", DataType::Int32, false),
+ Arc::new(ints) as ArrayRef,
+ ),
+ ]);
+
+ let schema = Schema::new(vec![Field::new(
+ "struct_array",
+ struct_array.data_type().clone(),
+ true,
+ )]);
+
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(struct_array)]).unwrap()
+ }
+
+ let record_batch = create_batch();
+ let record_batch_slice = record_batch.slice(1, 2);
Review Comment:
Yes, but it also applies the offset to the child data, which is the topic of that ticket. We have partially pushed down the offset, which at best is extremely confusing, but is likely just plain incorrect
--
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 commented on a diff in pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#discussion_r922407674
##########
arrow/src/ipc/writer.rs:
##########
@@ -1784,4 +1784,52 @@ mod tests {
assert_eq!(record_batch_slice, deserialized_batch);
}
+
+ #[test]
+ fn truncate_ipc_struct_array() {
+ fn create_batch() -> RecordBatch {
+ let strings: StringArray = [Some("foo"), None, Some("bar"), Some("baz")]
+ .into_iter()
+ .collect();
+ let ints: Int32Array =
+ [Some(0), Some(2), None, Some(1)].into_iter().collect();
+
+ let struct_array = StructArray::from(vec![
+ (
+ Field::new("s", DataType::Utf8, true),
+ Arc::new(strings) as ArrayRef,
+ ),
+ (
+ Field::new("c", DataType::Int32, false),
+ Arc::new(ints) as ArrayRef,
+ ),
+ ]);
+
+ let schema = Schema::new(vec![Field::new(
+ "struct_array",
+ struct_array.data_type().clone(),
+ true,
+ )]);
+
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(struct_array)]).unwrap()
+ }
+
+ let record_batch = create_batch();
+ let record_batch_slice = record_batch.slice(1, 2);
Review Comment:
There is currently an odd hack in Array::slice for StructArray that pushes down the offset to the child arrays. You will need to manually construct the ArrayData for the StructArray in order to properly test the case of a StructArray with a non-zero offset.
Edit: `StructArray::from(array.data().slice(1, 2))` might also work
--
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 #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#issuecomment-1185885756
Benchmark runs are scheduled for baseline = a7181ddbda801ba021398fb264cda394d64bff85 and contender = 32867e332ba901542a6adc5c81b61bc66002c751. 32867e332ba901542a6adc5c81b61bc66002c751 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/d561e647c1c74498a81f0240f14f3ac8...6bae1359f53d4475bff69bd691ffdd5b/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/9a56ac3f06564c038380a9d273d6ee0c...85e80489e9354efea11fc5b4fa093ed3/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/94a8effdc51440088977bb1f3d7321d0...649abcd058954efab36bc5a105714ea0/)
[Skipped :warning: Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5c0ad4c48a514ca3aef1afab75914123...026346993fe3422e889299f7212a0e00/)
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
[GitHub] [arrow-rs] viirya commented on pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
viirya commented on PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#issuecomment-1185887978
Thanks @tustvold
--
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 commented on a diff in pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#discussion_r922407674
##########
arrow/src/ipc/writer.rs:
##########
@@ -1784,4 +1784,52 @@ mod tests {
assert_eq!(record_batch_slice, deserialized_batch);
}
+
+ #[test]
+ fn truncate_ipc_struct_array() {
+ fn create_batch() -> RecordBatch {
+ let strings: StringArray = [Some("foo"), None, Some("bar"), Some("baz")]
+ .into_iter()
+ .collect();
+ let ints: Int32Array =
+ [Some(0), Some(2), None, Some(1)].into_iter().collect();
+
+ let struct_array = StructArray::from(vec![
+ (
+ Field::new("s", DataType::Utf8, true),
+ Arc::new(strings) as ArrayRef,
+ ),
+ (
+ Field::new("c", DataType::Int32, false),
+ Arc::new(ints) as ArrayRef,
+ ),
+ ]);
+
+ let schema = Schema::new(vec![Field::new(
+ "struct_array",
+ struct_array.data_type().clone(),
+ true,
+ )]);
+
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(struct_array)]).unwrap()
+ }
+
+ let record_batch = create_batch();
+ let record_batch_slice = record_batch.slice(1, 2);
Review Comment:
There is currently an odd hack in Array::slice for StructArray that pushes down the offset to the child arrays. You will need to manually construct the ArrayData for the StructArray in order to properly test the case of a StructArray with a non-zero offset. See https://github.com/apache/arrow-rs/issues/1750 for more information
--
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 commented on a diff in pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#discussion_r922407674
##########
arrow/src/ipc/writer.rs:
##########
@@ -1784,4 +1784,52 @@ mod tests {
assert_eq!(record_batch_slice, deserialized_batch);
}
+
+ #[test]
+ fn truncate_ipc_struct_array() {
+ fn create_batch() -> RecordBatch {
+ let strings: StringArray = [Some("foo"), None, Some("bar"), Some("baz")]
+ .into_iter()
+ .collect();
+ let ints: Int32Array =
+ [Some(0), Some(2), None, Some(1)].into_iter().collect();
+
+ let struct_array = StructArray::from(vec![
+ (
+ Field::new("s", DataType::Utf8, true),
+ Arc::new(strings) as ArrayRef,
+ ),
+ (
+ Field::new("c", DataType::Int32, false),
+ Arc::new(ints) as ArrayRef,
+ ),
+ ]);
+
+ let schema = Schema::new(vec![Field::new(
+ "struct_array",
+ struct_array.data_type().clone(),
+ true,
+ )]);
+
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(struct_array)]).unwrap()
+ }
+
+ let record_batch = create_batch();
+ let record_batch_slice = record_batch.slice(1, 2);
Review Comment:
There is currently an odd hack in Array::slice for StructArray that pushes down the offset to the child arrays. You will need to manually construct the ArrayData for the StructArray in order to properly test the case of a StructArray with a non-zero offset. See https://github.com/apache/arrow-rs/issues/1750 for more information
Edit: `StructArray::from(array.data().slice(1, 2))` might also work
--
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 commented on a diff in pull request #2083: Add IPC truncation test case for StructArray
Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2083:
URL: https://github.com/apache/arrow-rs/pull/2083#discussion_r922407674
##########
arrow/src/ipc/writer.rs:
##########
@@ -1784,4 +1784,52 @@ mod tests {
assert_eq!(record_batch_slice, deserialized_batch);
}
+
+ #[test]
+ fn truncate_ipc_struct_array() {
+ fn create_batch() -> RecordBatch {
+ let strings: StringArray = [Some("foo"), None, Some("bar"), Some("baz")]
+ .into_iter()
+ .collect();
+ let ints: Int32Array =
+ [Some(0), Some(2), None, Some(1)].into_iter().collect();
+
+ let struct_array = StructArray::from(vec![
+ (
+ Field::new("s", DataType::Utf8, true),
+ Arc::new(strings) as ArrayRef,
+ ),
+ (
+ Field::new("c", DataType::Int32, false),
+ Arc::new(ints) as ArrayRef,
+ ),
+ ]);
+
+ let schema = Schema::new(vec![Field::new(
+ "struct_array",
+ struct_array.data_type().clone(),
+ true,
+ )]);
+
+ RecordBatch::try_new(Arc::new(schema), vec![Arc::new(struct_array)]).unwrap()
+ }
+
+ let record_batch = create_batch();
+ let record_batch_slice = record_batch.slice(1, 2);
Review Comment:
There is currently an odd hack in ArrayData::slice for StructArray that pushes down the offset to the child arrays. You will need to manually construct the ArrayData for the StructArray in order to properly test the case of a StructArray with a non-zero offset. See https://github.com/apache/arrow-rs/issues/1750 for more information
--
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