blackmwk commented on code in PR #3514:
URL: https://github.com/apache/iceberg-rust/pull/3514#discussion_r4386662581
##########
crates/iceberg/src/arrow/reader/options.rs:
##########
@@ -15,70 +15,302 @@
// specific language governing permissions and limitations
// under the License.
-//! Tunables for Parquet file I/O used by `ArrowReader`.
-
+use serde::{Deserialize, Serialize};
use typed_builder::TypedBuilder;
-use super::{
- DEFAULT_METADATA_SIZE_HINT, DEFAULT_RANGE_COALESCE_BYTES,
DEFAULT_RANGE_FETCH_CONCURRENCY,
-};
+use super::{ArrowReader, ParquetReadOptions};
+use crate::arrow::caching_delete_file_loader::CachingDeleteFileLoader;
+use crate::io::FileIO;
+use crate::runtime::Runtime;
+use crate::util::available_parallelism;
+
+/// Configures Arrow reads independently of file planning and local I/O
resources.
+///
+/// ```no_run
+/// # use iceberg::{Result, arrow::{ArrowReaderOptions, ParquetReadOptions},
table::Table};
+/// # async fn read(table: &Table) -> Result<()> {
+/// let options = ArrowReaderOptions::builder()
+/// .with_batch_size(Some(1024))
+/// .with_parquet_options(
+/// ParquetReadOptions::builder()
+/// .with_metadata_size_hint(Some(512 * 1024))
+/// .build(),
+/// )
+/// .build();
+/// let stream = table.scan().build()?.to_arrow(options).await?;
+/// # Ok(())
+/// # }
+/// ```
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, TypedBuilder)]
+#[serde(default)]
+#[builder(field_defaults(default, setter(prefix = "with_")))]
+pub struct ArrowReaderOptions {
+ /// Desired batch size, or the reader default when unset.
+ batch_size: Option<usize>,
+ /// Maximum in-flight data files, or execution-host parallelism when unset.
+ data_file_concurrency_limit: Option<usize>,
+ /// Whether to prune row groups using their statistics.
+ #[builder(default = true)]
+ row_group_filtering_enabled: bool,
+ /// Whether to use page indexes for row selection.
+ row_selection_enabled: bool,
+ /// Whether to use bloom filters for row-group pruning.
+ bloom_filter_enabled: bool,
+ /// I/O tuning applied when reading Parquet files.
+ parquet_options: ParquetReadOptions,
+}
+
+impl Default for ArrowReaderOptions {
+ fn default() -> Self {
+ Self::builder().build()
+ }
+}
+
+/// Builder to create ArrowReader
+pub struct ArrowReaderBuilder {
Review Comment:
Let's keep the builder in mod.rs, it's odd to put it in an option mod.
##########
crates/iceberg/src/arrow/reader/parquet_options.rs:
##########
@@ -0,0 +1,95 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Tunables for Parquet file I/O used by `ArrowReader`.
+
+use serde::{Deserialize, Serialize};
+use typed_builder::TypedBuilder;
+
+use super::{
+ DEFAULT_METADATA_SIZE_HINT, DEFAULT_RANGE_COALESCE_BYTES,
DEFAULT_RANGE_FETCH_CONCURRENCY,
+};
+
+/// Options for tuning Parquet file I/O.
+#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize, TypedBuilder)]
+#[serde(default)]
+#[builder(field_defaults(setter(prefix = "with_")))]
+pub struct ParquetReadOptions {
Review Comment:
Since we are going to embed it in ArrowReaderOptions, I think we should also
put it in option mod.
##########
crates/iceberg/src/arrow/reader/parquet_options.rs:
##########
@@ -0,0 +1,95 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Tunables for Parquet file I/O used by `ArrowReader`.
+
+use serde::{Deserialize, Serialize};
+use typed_builder::TypedBuilder;
+
+use super::{
+ DEFAULT_METADATA_SIZE_HINT, DEFAULT_RANGE_COALESCE_BYTES,
DEFAULT_RANGE_FETCH_CONCURRENCY,
+};
+
+/// Options for tuning Parquet file I/O.
+#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize, TypedBuilder)]
+#[serde(default)]
+#[builder(field_defaults(setter(prefix = "with_")))]
+pub struct ParquetReadOptions {
+ /// Number of bytes to prefetch for parsing the Parquet metadata.
+ ///
+ /// This hint can help reduce the number of fetch requests. For more
details see the
+ /// [ParquetMetaDataReader
documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint).
+ ///
+ /// Defaults to 512 KiB, matching DataFusion's default
`ParquetOptions::metadata_size_hint`.
+ #[builder(default = Some(DEFAULT_METADATA_SIZE_HINT))]
+ pub(crate) metadata_size_hint: Option<usize>,
Review Comment:
We should remove the `pub(crate)` modifier and expose pub get methods.
##########
crates/iceberg/src/arrow/reader/parquet_options.rs:
##########
@@ -0,0 +1,95 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Tunables for Parquet file I/O used by `ArrowReader`.
+
+use serde::{Deserialize, Serialize};
+use typed_builder::TypedBuilder;
+
+use super::{
+ DEFAULT_METADATA_SIZE_HINT, DEFAULT_RANGE_COALESCE_BYTES,
DEFAULT_RANGE_FETCH_CONCURRENCY,
+};
+
+/// Options for tuning Parquet file I/O.
+#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize, TypedBuilder)]
+#[serde(default)]
+#[builder(field_defaults(setter(prefix = "with_")))]
+pub struct ParquetReadOptions {
+ /// Number of bytes to prefetch for parsing the Parquet metadata.
+ ///
+ /// This hint can help reduce the number of fetch requests. For more
details see the
+ /// [ParquetMetaDataReader
documentation](https://docs.rs/parquet/latest/parquet/file/metadata/struct.ParquetMetaDataReader.html#method.with_prefetch_hint).
+ ///
+ /// Defaults to 512 KiB, matching DataFusion's default
`ParquetOptions::metadata_size_hint`.
+ #[builder(default = Some(DEFAULT_METADATA_SIZE_HINT))]
Review Comment:
It would be more natures to strip option.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
