Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add nested struct supports #1519

Merged
merged 5 commits into from
Jul 15, 2023
Merged

feat: add nested struct supports #1519

merged 5 commits into from
Jul 15, 2023

Conversation

haruband
Copy link
Contributor

@haruband haruband commented Jul 6, 2023

Description

This PR adds nested struct supports.

Related Issue(s)

Documentation

@github-actions github-actions bot added binding/rust Issues for the Rust crate rust labels Jul 6, 2023
@github-actions
Copy link

github-actions bot commented Jul 6, 2023

ACTION NEEDED

delta-rs follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

@haruband haruband changed the title Add nested struct supports feat: add nested struct supports Jul 6, 2023
Copy link
Member

@rtyler rtyler left a comment

Choose a reason for hiding this comment

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

I think this mostly looks good and can be merged with some minor tweaks. If you don't happen to get to it, i'll knock it out tomorrow

Comment on lines 496 to 498
fn can_cast_batch(from_schema: &ArrowSchema, to_schema: &ArrowSchema) -> bool {
can_cast_batch_fields(from_schema.fields(), to_schema.fields())
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fn can_cast_batch(from_schema: &ArrowSchema, to_schema: &ArrowSchema) -> bool {
can_cast_batch_fields(from_schema.fields(), to_schema.fields())
}

Since there is only one callsite of this method and it is not a public API, I figure it makes sense to just remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you. I pushed new commit.
Thanks for the review.

This commit allows to check and cast record batches with column of
type nested struct.
This test checks if a field in nested struct is casted.
@rtyler rtyler enabled auto-merge (squash) July 15, 2023 05:10
@rtyler rtyler self-assigned this Jul 15, 2023
@rtyler rtyler merged commit 4a4aaa9 into delta-io:main Jul 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/rust Issues for the Rust crate rust
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add nested struct supports
2 participants