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/06/27 23:04:24 UTC
[GitHub] [arrow-datafusion] alamb opened a new pull request, #2802: Correct schema nullability declaration in tests
alamb opened a new pull request, #2802:
URL: https://github.com/apache/arrow-datafusion/pull/2802
# Which issue does this PR close?
Part of https://github.com/apache/arrow-datafusion/pull/2778
# Rationale for this change
https://github.com/apache/arrow-rs/issues/1888 (added in `arrow 17.0.0`) added validation to `RecordBatch` if the schema's declared nullability is different than its actual nullability it throws a runtime error.
# What changes are included in this PR?
Correct declared schema nullability in DataFusion tests .
There are no tests in this PR -- they are covered in https://github.com/apache/arrow-datafusion/pull/2778
# Are there any user-facing changes?
No, it is a test only change
--
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-datafusion] alamb commented on a diff in pull request #2802: Correct schema nullability declaration in tests
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2802:
URL: https://github.com/apache/arrow-datafusion/pull/2802#discussion_r907870656
##########
datafusion/core/src/physical_optimizer/aggregate_statistics.rs:
##########
@@ -276,8 +276,8 @@ mod tests {
/// Mock data using a MemoryExec which has an exact count statistic
fn mock_data() -> Result<Arc<MemoryExec>> {
let schema = Arc::new(Schema::new(vec![
- Field::new("a", DataType::Int32, false),
- Field::new("b", DataType::Int32, false),
+ Field::new("a", DataType::Int32, true),
+ Field::new("b", DataType::Int32, true),
Review Comment:
This is a pretty easy to understand example of the issue -- prior to this PR, the fields `"a"` and `"b"` are declared as `"nullable=false"` but then 5 lines lower `NULL` data is inserted 🤦
```rust
let batch = RecordBatch::try_new(
Arc::clone(&schema),
vec![
Arc::new(Int32Array::from(vec![Some(1), Some(2), None])),
Arc::new(Int32Array::from(vec![Some(4), None, Some(6)])),
],
)?;
```
Now that `RecordBatch::try_new` validates the nullability, the schema must match the data otherwise an error results
--
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-datafusion] codecov-commenter commented on pull request #2802: Correct schema nullability declaration in tests
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2802:
URL: https://github.com/apache/arrow-datafusion/pull/2802#issuecomment-1168039981
# [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/2802?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 [#2802](https://codecov.io/gh/apache/arrow-datafusion/pull/2802?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b6e898c) into [master](https://codecov.io/gh/apache/arrow-datafusion/commit/533e2b4c72da854f4d9e4a9e2d3d7c35814b7ffd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (533e2b4) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head b6e898c differs from pull request most recent head 5dc0e51. Consider uploading reports for the commit 5dc0e51 to get more accurate results
```diff
@@ Coverage Diff @@
## master #2802 +/- ##
==========================================
- Coverage 85.11% 85.11% -0.01%
==========================================
Files 273 273
Lines 48240 48240
==========================================
- Hits 41060 41059 -1
- Misses 7180 7181 +1
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-datafusion/pull/2802?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...fusion/physical-expr/src/aggregate/sum\_distinct.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9hZ2dyZWdhdGUvc3VtX2Rpc3RpbmN0LnJz) | `92.66% <ø> (ø)` | |
| [datafusion/physical-expr/src/expressions/cast.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy9jYXN0LnJz) | `98.00% <ø> (ø)` | |
| [datafusion/physical-expr/src/expressions/mod.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy9tb2QucnM=) | `100.00% <ø> (ø)` | |
| [...tafusion/physical-expr/src/expressions/try\_cast.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy90cnlfY2FzdC5ycw==) | `98.75% <ø> (ø)` | |
| [datafusion/core/src/datasource/listing/helpers.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9kYXRhc291cmNlL2xpc3RpbmcvaGVscGVycy5ycw==) | `95.30% <100.00%> (ø)` | |
| [...ore/src/physical\_optimizer/aggregate\_statistics.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9waHlzaWNhbF9vcHRpbWl6ZXIvYWdncmVnYXRlX3N0YXRpc3RpY3MucnM=) | `100.00% <100.00%> (ø)` | |
| [datafusion/core/src/physical\_plan/hash\_join.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9waHlzaWNhbF9wbGFuL2hhc2hfam9pbi5ycw==) | `94.68% <100.00%> (ø)` | |
| [datafusion/core/src/physical\_plan/memory.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9waHlzaWNhbF9wbGFuL21lbW9yeS5ycw==) | `96.03% <100.00%> (ø)` | |
| [...afusion/physical-expr/src/aggregate/correlation.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9hZ2dyZWdhdGUvY29ycmVsYXRpb24ucnM=) | `97.97% <100.00%> (ø)` | |
| [...tafusion/physical-expr/src/aggregate/covariance.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9hZ2dyZWdhdGUvY292YXJpYW5jZS5ycw==) | `98.71% <100.00%> (ø)` | |
| ... and [7 more](https://codecov.io/gh/apache/arrow-datafusion/pull/2802/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-datafusion/pull/2802?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-datafusion/pull/2802?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 [533e2b4...5dc0e51](https://codecov.io/gh/apache/arrow-datafusion/pull/2802?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-datafusion] andygrove merged pull request #2802: Correct schema nullability declaration in tests
Posted by GitBox <gi...@apache.org>.
andygrove merged PR #2802:
URL: https://github.com/apache/arrow-datafusion/pull/2802
--
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