Skip to content

ARROW-10656: [Rust] Allow schema validation to ignore field names and only check data types on new batch - #8988

Closed
nevi-me wants to merge 1 commit into
apache:masterfrom
nevi-me:ARROW-10656
Closed

ARROW-10656: [Rust] Allow schema validation to ignore field names and only check data types on new batch#8988
nevi-me wants to merge 1 commit into
apache:masterfrom
nevi-me:ARROW-10656

Conversation

@nevi-me

@nevi-menevi-me commented Dec 22, 2020

Copy link
Copy Markdown
Contributor

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

@nevi-menevi-me changed the title ARROW-10655: [Rust] Allow eaner schema validation on new batchARROW-10656: [Rust] Allow eaner schema validation on new batchDec 22, 2020
@nevi-me

nevi-me commented Dec 22, 2020

Copy link
Copy Markdown
ContributorAuthor

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

@nevi-menevi-me changed the title ARROW-10656: [Rust] Allow eaner schema validation on new batchARROW-10656: [Rust] Allow leaner schema validation on new batchDec 22, 2020
@codecov-io

codecov-io commented Dec 22, 2020•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #8988 (78b560c) into master (51672b2) will decrease coverage by 0.05%.
The diff coverage is 26.00%.

Impacted file tree graph

@@ 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 FilesCoverage Δ
rust/arrow/src/datatypes.rs75.02% <0.00%> (-1.43%)⬇️
rust/arrow/src/record_batch.rs76.06% <38.23%> (-12.23%)⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51672b2...78b560c. Read the comment docs.

@github-actions

github-actionsBot commented Dec 22, 2020

Copy link
Copy Markdown

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

@ch-sc

ch-sc commented Dec 27, 2020

Copy link
Copy Markdown
Contributor

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

@nevi-menevi-me mentioned this pull request Dec 29, 2020
Closed
alamb
alamb approved these changes Dec 31, 2020
View reviewed changes

@alambalamb left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @nevi-me ! I went through this and it looks good. I also merged the this code into apache/master locally and re-ran all tests. I think this is good to go.

@alamb

alamb commented Dec 31, 2020

Copy link
Copy Markdown
Contributor

The full set of Rust CI tests did not run on this PR :(

Can you please rebase this PR against apache/master to pick up the changes in #9056 so that they do?

I apologize for the inconvenience.

@nevi-me

nevi-me commented Jan 1, 2021

Copy link
Copy Markdown
ContributorAuthor

Done rebasing @alamb

jorgecarleitao
jorgecarleitao approved these changes Jan 1, 2021
View reviewed changes

@jorgecarleitaojorgecarleitao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@alambalamb changed the title ARROW-10656: [Rust] Allow leaner schema validation on new batchARROW-10656: [Rust] Allow schema validation to ignore field names and only check data types on new batchJan 1, 2021
@alambalamb closed this in 118f462Jan 1, 2021
@nevi-me
nevi-me deleted the ARROW-10656 branch January 2, 2021 05:48
nevi-me
nevi-me commented Jan 2, 2021
View reviewed changes
Comment threadrust/arrow/src/record_batch.rs
Ok(RecordBatch { schema, columns })
}

pub fn try_new_with_options(

@nevi-menevi-meJan 2, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@nevi-me
nevi-me restored the ARROW-10656 branch January 2, 2021 05:51
@asfimportasfimport mentioned this pull request Jan 2, 2021
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alambalambalamb approved these changes
@jorgecarleitaojorgecarleitaojorgecarleitao approved these changes

Assignees

No one assigned

Labels

Component: Rust

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nevi-me@codecov-io@ch-sc@alamb@jorgecarleitao