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

refactor: remove remaining uses of arrow_array and use reexport in arrow instead #14528

Merged

Conversation

Chen-Yuan-Lai
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

As #14503 (review) says, arrow_array should be removed and be replaced with reexport in arrow as much as possible.

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added sql SQL Planner physical-expr Physical Expressions core Core DataFusion crate common Related to common crate functions labels Feb 6, 2025
@Chen-Yuan-Lai Chen-Yuan-Lai force-pushed the use_reexport_in_arrow_instead_of_arrow_array branch from 78f7847 to 4bd8dac Compare February 7, 2025 01:49
@Chen-Yuan-Lai Chen-Yuan-Lai force-pushed the use_reexport_in_arrow_instead_of_arrow_array branch from 4bd8dac to 327bc78 Compare February 7, 2025 01:57
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

A very nice simplification @Chen-Yuan-Lai -- thank you 🙏

Screenshot 2025-02-07 at 1 54 46 PM

@@ -51,7 +51,6 @@ apache-avro = { version = "0.17", default-features = false, features = [
"zstandard",
], optional = true }
arrow = { workspace = true }
arrow-array = { workspace = true }
arrow-ipc = { workspace = true }
arrow-schema = { workspace = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, looks like there is one PR left -- arrow-schema 🤔

Any chance you want another fun cleanup PR @Chen-Yuan-Lai :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alamb Sure 💪

@alamb alamb merged commit faa8c1b into apache:main Feb 8, 2025
27 checks passed
@alamb
Copy link
Contributor

alamb commented Feb 8, 2025

Thanks again @Chen-Yuan-Lai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Related to common crate core Core DataFusion crate functions physical-expr Physical Expressions sql SQL Planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use NullBufferBuilder instead of BooleanBufferBuilder for creating Null masks
2 participants