Skip to content

make builder public under experimental #7825

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 30, 2025
Merged

make builder public under experimental #7825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6801df
make builder pub
XiangpengHao Jun 29, 2025
e57c5bc
add a comment
XiangpengHao Jun 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions parquet/src/arrow/array_reader/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ use crate::errors::{ParquetError, Result};
use crate::schema::types::{ColumnDescriptor, ColumnPath, Type};

/// Builds [`ArrayReader`]s from parquet schema, projection mask, and RowGroups reader
pub(crate) struct ArrayReaderBuilder<'a> {
pub struct ArrayReaderBuilder<'a> {
row_groups: &'a dyn RowGroups,
}

impl<'a> ArrayReaderBuilder<'a> {
pub(crate) fn new(row_groups: &'a dyn RowGroups) -> Self {
pub fn new(row_groups: &'a dyn RowGroups) -> Self {
Self { row_groups }
}

Expand Down
3 changes: 2 additions & 1 deletion parquet/src/arrow/array_reader/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ mod struct_array;
#[cfg(test)]
mod test_util;

pub(crate) use builder::ArrayReaderBuilder;
// Note that this crate is public under the `experimental` feature flag.
Copy link
Contributor Author

@XiangpengHao XiangpengHao Jun 29, 2025

Choose a reason for hiding this comment

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

I saw your comment #7521 (comment) @alamb , so added a line here to explain that this crate is public.

maybe it's also time to graduate this crate to remove experimental?

Copy link
Contributor

@alamb alamb Jun 30, 2025

Choose a reason for hiding this comment

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

If we are going to make it non experimental we I think we need to add tests (e.g. doctest) or something that shows how the public API is being used so we don't break it in the future.

I need to think about what the implications of making it pub are -- it may make more sense to do that if/when we have finally sorted out how what it takes to turn on filter pushdown

pub use builder::ArrayReaderBuilder;
pub use byte_array::make_byte_array_reader;
pub use byte_array_dictionary::make_byte_array_dictionary_reader;
#[allow(unused_imports)] // Only used for benchmarks
Expand Down
Loading