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

Remove SchemaBuilder dependency from StructArray constructors #6138

Closed
Rafferty97 opened this issue Jul 27, 2024 · 0 comments · Fixed by #6139
Closed

Remove SchemaBuilder dependency from StructArray constructors #6138

Rafferty97 opened this issue Jul 27, 2024 · 0 comments · Fixed by #6139
Labels
arrow Changes to the arrow crate

Comments

@Rafferty97
Copy link
Contributor

Summary

Three functions in array/struct_array.rs use SchemaBuilder in the course of building a StructArray. However, in all cases they just extract the fields from the schema and throw out the empty metadata. It seems it would be more straightforward to just collect the fields into a Vec directly.

I had considered that this might be intentional, perhaps in case the implementation of SchemaBuilder is ever changed, for example to collect into something other than a Vec. If so, I'd appreciate a little more context because as it stands this abstraction seems more confusing than useful.

Proposal

In the three mentioned functions, collect into a Vec<Field> directly and then .into() it into a Fields. This eliminates the dependency on SchemaBuilder, making the code easier to read in isolation.

@alamb alamb added the arrow Changes to the arrow crate label Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants