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/22 04:19:09 UTC

[GitHub] [arrow] nevi-me opened a new pull request #8988: ARROW-10655: [Rust] Allow eaner schema validation on new batch

nevi-me opened a new pull request #8988:
URL: https://github.com/apache/arrow/pull/8988


   This adds the option to create a new record batch with less strict validation for list field names.
   The default behaviour is preserved.


----------------------------------------------------------------
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] ch-sc commented on pull request #8988: ARROW-10656: [Rust] Allow leaner schema validation on new batch

Posted by GitBox <gi...@apache.org>.
ch-sc commented on pull request #8988:
URL: https://github.com/apache/arrow/pull/8988#issuecomment-751517720


   Looks good to me @nevi-me. Thank you!


----------------------------------------------------------------
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] nevi-me commented on pull request #8988: ARROW-10656: [Rust] Allow leaner schema validation on new batch

Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #8988:
URL: https://github.com/apache/arrow/pull/8988#issuecomment-753271373


   Done rebasing @alamb 


----------------------------------------------------------------
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 #8988: ARROW-10656: [Rust] Allow leaner schema validation on new batch

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


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


----------------------------------------------------------------
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 closed pull request #8988: ARROW-10656: [Rust] Allow schema validation to ignore field names and only check data types on new batch

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


   


----------------------------------------------------------------
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] nevi-me commented on pull request #8988: ARROW-10656: [Rust] Allow eaner schema validation on new batch

Posted by GitBox <gi...@apache.org>.
nevi-me commented on pull request #8988:
URL: https://github.com/apache/arrow/pull/8988#issuecomment-749329281


   Hi @ch-sc , please have a look at the approach that I've taken, and let me know if it meets your use-case.
   
   If you're happy with the PR, I can then document the new functions, and add unit tests, then get it reviewed.
   
   Thanks


----------------------------------------------------------------
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] nevi-me commented on a change in pull request #8988: ARROW-10656: [Rust] Allow schema validation to ignore field names and only check data types on new batch

Posted by GitBox <gi...@apache.org>.
nevi-me commented on a change in pull request #8988:
URL: https://github.com/apache/arrow/pull/8988#discussion_r550847402



##########
File path: rust/arrow/src/record_batch.rs
##########
@@ -75,6 +75,25 @@ impl RecordBatch {
     /// # }
     /// ```
     pub fn try_new(schema: SchemaRef, columns: Vec<ArrayRef>) -> Result<Self> {
+        let options = RecordBatchOptions::default();
+        Self::validate_new_batch(&schema, columns.as_slice(), &options)?;
+        Ok(RecordBatch { schema, columns })
+    }
+
+    pub fn try_new_with_options(

Review comment:
       @alamb @jorgecarleitao I must have done something wrong on the rebase, I had added doc comments for this function, and added tests :(




----------------------------------------------------------------
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 #8988: ARROW-10656: [Rust] Allow leaner schema validation on new batch

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


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=h1) Report
   > Merging [#8988](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=desc) (3c95773) into [master](https://codecov.io/gh/apache/arrow/commit/dd5fe7095bb662236e27d3343eb82bc4375f93ef?el=desc) (dd5fe70) will **decrease** coverage by `0.05%`.
   > The diff coverage is `26.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/8988/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #8988      +/-   ##
   ==========================================
   - Coverage   82.61%   82.56%   -0.06%     
   ==========================================
     Files         202      202              
     Lines       50052    50091      +39     
   ==========================================
   + Hits        41350    41357       +7     
   - Misses       8702     8734      +32     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/datatypes.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvZGF0YXR5cGVzLnJz) | `75.02% <0.00%> (-1.43%)` | :arrow_down: |
   | [rust/arrow/src/record\_batch.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvcmVjb3JkX2JhdGNoLnJz) | `76.06% <38.23%> (-12.23%)` | :arrow_down: |
   | [rust/parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow/pull/8988/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/8988?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/8988?src=pr&el=footer). Last update [709f20d...3c95773](https://codecov.io/gh/apache/arrow/pull/8988?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] alamb commented on pull request #8988: ARROW-10656: [Rust] Allow leaner schema validation on new batch

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


   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] codecov-io commented on pull request #8988: ARROW-10656: [Rust] Allow leaner schema validation on new batch

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


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=h1) Report
   > Merging [#8988](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=desc) (883f735) into [master](https://codecov.io/gh/apache/arrow/commit/4c48539c936e74aed266221d9fc76f377700216d?el=desc) (4c48539) will **decrease** coverage by `0.49%`.
   > The diff coverage is `48.23%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/8988/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #8988      +/-   ##
   ==========================================
   - Coverage   83.09%   82.60%   -0.50%     
   ==========================================
     Files         200      200              
     Lines       49076    49714     +638     
   ==========================================
   + Hits        40782    41065     +283     
   - Misses       8294     8649     +355     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow-flight/src/utils.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy1mbGlnaHQvc3JjL3V0aWxzLnJz) | `0.00% <0.00%> (ø)` | |
   | [rust/arrow/src/datatypes.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvZGF0YXR5cGVzLnJz) | `75.02% <0.00%> (-1.43%)` | :arrow_down: |
   | [rust/arrow/src/ipc/gen/SparseTensor.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvaXBjL2dlbi9TcGFyc2VUZW5zb3IucnM=) | `0.00% <0.00%> (ø)` | |
   | [rust/arrow/src/ipc/gen/Tensor.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvaXBjL2dlbi9UZW5zb3IucnM=) | `0.00% <0.00%> (ø)` | |
   | [rust/arrow/src/ipc/convert.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvaXBjL2NvbnZlcnQucnM=) | `91.90% <27.27%> (-1.11%)` | :arrow_down: |
   | [rust/arrow/src/record\_batch.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvcmVjb3JkX2JhdGNoLnJz) | `76.06% <38.23%> (-12.23%)` | :arrow_down: |
   | [rust/arrow/src/ipc/gen/File.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvaXBjL2dlbi9GaWxlLnJz) | `40.94% <39.28%> (-2.20%)` | :arrow_down: |
   | [rust/parquet/src/arrow/schema.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9wYXJxdWV0L3NyYy9hcnJvdy9zY2hlbWEucnM=) | `90.62% <40.00%> (-0.44%)` | :arrow_down: |
   | [rust/arrow/src/ipc/gen/Message.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvaXBjL2dlbi9NZXNzYWdlLnJz) | `32.03% <42.10%> (+2.21%)` | :arrow_up: |
   | [rust/arrow/src/ipc/reader.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvaXBjL3JlYWRlci5ycw==) | `82.86% <50.00%> (-0.84%)` | :arrow_down: |
   | ... and [4 more](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/8988?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/8988?src=pr&el=footer). Last update [a2e7d3a...883f735](https://codecov.io/gh/apache/arrow/pull/8988?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] codecov-io edited a comment on pull request #8988: ARROW-10656: [Rust] Allow leaner schema validation on new batch

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


   # [Codecov](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=h1) Report
   > Merging [#8988](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=desc) (78b560c) into [master](https://codecov.io/gh/apache/arrow/commit/51672b28e97f19f70de0f0a8800c40ee9bb939d3?el=desc) (51672b2) will **decrease** coverage by `0.05%`.
   > The diff coverage is `26.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/arrow/pull/8988/graphs/tree.svg?width=650&height=150&src=pr&token=LpTCFbqVT1)](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #8988      +/-   ##
   ==========================================
   - Coverage   82.61%   82.56%   -0.06%     
   ==========================================
     Files         202      202              
     Lines       50048    50087      +39     
   ==========================================
   + Hits        41347    41353       +6     
   - Misses       8701     8734      +33     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow/pull/8988?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [rust/arrow/src/datatypes.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvZGF0YXR5cGVzLnJz) | `75.02% <0.00%> (-1.43%)` | :arrow_down: |
   | [rust/arrow/src/record\_batch.rs](https://codecov.io/gh/apache/arrow/pull/8988/diff?src=pr&el=tree#diff-cnVzdC9hcnJvdy9zcmMvcmVjb3JkX2JhdGNoLnJz) | `76.06% <38.23%> (-12.23%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow/pull/8988?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/8988?src=pr&el=footer). Last update [51672b2...78b560c](https://codecov.io/gh/apache/arrow/pull/8988?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