You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "comphead (via GitHub)" <gi...@apache.org> on 2026/03/21 19:37:36 UTC
[PR] chore: Experiments with `native_datafusion` scan optimizations [datafusion-comet]
comphead opened a new pull request, #3755:
URL: https://github.com/apache/datafusion-comet/pull/3755
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Part of #3748
## Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
## How are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: cache Parquet metadata in `native_datafusion` scan [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #3755:
URL: https://github.com/apache/datafusion-comet/pull/3755#issuecomment-4111630869
> Is this scope right? The comment says "across spark partitions" but if its lifecycle is tied to a scan object, we don't actually get that benefit. We would need to move its lifecycle up to some sort of context object, but I don't know what the proper key is for that.
hm, it was partially rolled back because of Spark failed test `partitioned table is cached when partition pruning is true *** FAILED *** (1 second, 574 milliseconds)`
The test delete the file from the disk and expects `FileNotFoundException` and our cached manager doesn't track that and responds as if file still exists. I wanna keep the caching factory and use in other context object like you mentioned but with tracking deleted files
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: `native_datafusion` scan optimizations part1 [datafusion-comet]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on code in PR #3755:
URL: https://github.com/apache/datafusion-comet/pull/3755#discussion_r2975397771
##########
native/core/src/parquet/parquet_read_cached_factory.rs:
##########
@@ -0,0 +1,149 @@
+// 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.
+
+//! A `ParquetFileReaderFactory` that caches parquet footer metadata across
+//! partitions within a single scan. When multiple Spark partitions read from
+//! the same parquet file (different row group ranges), this avoids redundant
+//! footer reads and parsing.
+//!
+//! The cache is scoped to the factory instance (one per scan), not global,
+//! so it does not persist across queries.
+//!
+//! Uses `tokio::sync::OnceCell` per file path so that concurrent partitions
+//! wait for the first reader to load the footer rather than all racing.
+
+use bytes::Bytes;
+use datafusion::common::Result as DataFusionResult;
+use datafusion::datasource::physical_plan::parquet::ParquetFileReaderFactory;
+use datafusion::physical_plan::metrics::{ExecutionPlanMetricsSet, MetricBuilder};
+use datafusion_datasource::PartitionedFile;
+use futures::future::BoxFuture;
+use futures::FutureExt;
+use object_store::path::Path;
+use object_store::ObjectStore;
+use parquet::arrow::arrow_reader::ArrowReaderOptions;
+use parquet::arrow::async_reader::{AsyncFileReader, ParquetObjectReader};
+use parquet::file::metadata::ParquetMetaData;
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::ops::Range;
+use std::sync::{Arc, Mutex};
+use tokio::sync::OnceCell;
+
+type MetadataCell = Arc<OnceCell<Arc<ParquetMetaData>>>;
+
+/// A `ParquetFileReaderFactory` that caches footer metadata by file path.
+/// The cache is scoped to this factory instance (shared across partitions
+/// within a single scan via Arc), not global.
+#[derive(Debug)]
+pub struct CachingParquetReaderFactory {
+ store: Arc<dyn ObjectStore>,
+ cache: Arc<Mutex<HashMap<Path, MetadataCell>>>,
+}
+
+impl CachingParquetReaderFactory {
+ pub fn new(store: Arc<dyn ObjectStore>) -> Self {
+ Self {
+ store,
+ cache: Arc::new(Mutex::new(HashMap::new())),
+ }
+ }
+}
+
+impl ParquetFileReaderFactory for CachingParquetReaderFactory {
+ fn create_reader(
+ &self,
+ partition_index: usize,
+ partitioned_file: PartitionedFile,
+ metadata_size_hint: Option<usize>,
+ metrics: &ExecutionPlanMetricsSet,
+ ) -> DataFusionResult<Box<dyn AsyncFileReader + Send>> {
+ let bytes_scanned = MetricBuilder::new(metrics).counter("bytes_scanned", partition_index);
+
+ let location = partitioned_file.object_meta.location.clone();
+
+ // Get or create the OnceCell for this file path
+ let cell = Arc::<OnceCell<Arc<ParquetMetaData>>>::clone(
+ self.cache
+ .lock()
+ .unwrap()
+ .entry(location.clone())
+ .or_insert_with(|| Arc::new(OnceCell::new())),
+ );
+
+ let mut inner = ParquetObjectReader::new(Arc::clone(&self.store), location.clone())
+ .with_file_size(partitioned_file.object_meta.size);
+
+ if let Some(hint) = metadata_size_hint {
+ inner = inner.with_footer_size_hint(hint);
+ }
+
+ Ok(Box::new(CachingParquetFileReader {
+ inner,
+ location,
+ cell,
+ bytes_scanned,
+ }))
+ }
+}
+
+struct CachingParquetFileReader {
+ inner: ParquetObjectReader,
+ location: Path,
+ cell: MetadataCell,
+ bytes_scanned: datafusion::physical_plan::metrics::Count,
+}
+
+impl AsyncFileReader for CachingParquetFileReader {
+ fn get_bytes(&mut self, range: Range<u64>) -> BoxFuture<'_, parquet::errors::Result<Bytes>> {
+ self.bytes_scanned.add((range.end - range.start) as usize);
+ self.inner.get_bytes(range)
+ }
+
+ fn get_byte_ranges(
Review Comment:
Do we have to implement this?
##########
native/core/src/parquet/parquet_read_cached_factory.rs:
##########
@@ -0,0 +1,149 @@
+// 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.
+
+//! A `ParquetFileReaderFactory` that caches parquet footer metadata across
+//! partitions within a single scan. When multiple Spark partitions read from
+//! the same parquet file (different row group ranges), this avoids redundant
+//! footer reads and parsing.
+//!
+//! The cache is scoped to the factory instance (one per scan), not global,
+//! so it does not persist across queries.
+//!
+//! Uses `tokio::sync::OnceCell` per file path so that concurrent partitions
+//! wait for the first reader to load the footer rather than all racing.
+
+use bytes::Bytes;
+use datafusion::common::Result as DataFusionResult;
+use datafusion::datasource::physical_plan::parquet::ParquetFileReaderFactory;
+use datafusion::physical_plan::metrics::{ExecutionPlanMetricsSet, MetricBuilder};
+use datafusion_datasource::PartitionedFile;
+use futures::future::BoxFuture;
+use futures::FutureExt;
+use object_store::path::Path;
+use object_store::ObjectStore;
+use parquet::arrow::arrow_reader::ArrowReaderOptions;
+use parquet::arrow::async_reader::{AsyncFileReader, ParquetObjectReader};
+use parquet::file::metadata::ParquetMetaData;
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::ops::Range;
+use std::sync::{Arc, Mutex};
+use tokio::sync::OnceCell;
+
+type MetadataCell = Arc<OnceCell<Arc<ParquetMetaData>>>;
+
+/// A `ParquetFileReaderFactory` that caches footer metadata by file path.
+/// The cache is scoped to this factory instance (shared across partitions
+/// within a single scan via Arc), not global.
+#[derive(Debug)]
+pub struct CachingParquetReaderFactory {
+ store: Arc<dyn ObjectStore>,
+ cache: Arc<Mutex<HashMap<Path, MetadataCell>>>,
+}
+
+impl CachingParquetReaderFactory {
+ pub fn new(store: Arc<dyn ObjectStore>) -> Self {
+ Self {
+ store,
+ cache: Arc::new(Mutex::new(HashMap::new())),
+ }
+ }
+}
+
+impl ParquetFileReaderFactory for CachingParquetReaderFactory {
+ fn create_reader(
+ &self,
+ partition_index: usize,
+ partitioned_file: PartitionedFile,
+ metadata_size_hint: Option<usize>,
+ metrics: &ExecutionPlanMetricsSet,
+ ) -> DataFusionResult<Box<dyn AsyncFileReader + Send>> {
+ let bytes_scanned = MetricBuilder::new(metrics).counter("bytes_scanned", partition_index);
+
+ let location = partitioned_file.object_meta.location.clone();
+
+ // Get or create the OnceCell for this file path
+ let cell = Arc::<OnceCell<Arc<ParquetMetaData>>>::clone(
+ self.cache
+ .lock()
+ .unwrap()
+ .entry(location.clone())
+ .or_insert_with(|| Arc::new(OnceCell::new())),
+ );
+
+ let mut inner = ParquetObjectReader::new(Arc::clone(&self.store), location.clone())
+ .with_file_size(partitioned_file.object_meta.size);
+
+ if let Some(hint) = metadata_size_hint {
+ inner = inner.with_footer_size_hint(hint);
+ }
+
+ Ok(Box::new(CachingParquetFileReader {
+ inner,
+ location,
+ cell,
+ bytes_scanned,
+ }))
+ }
+}
+
+struct CachingParquetFileReader {
+ inner: ParquetObjectReader,
+ location: Path,
+ cell: MetadataCell,
+ bytes_scanned: datafusion::physical_plan::metrics::Count,
+}
+
+impl AsyncFileReader for CachingParquetFileReader {
+ fn get_bytes(&mut self, range: Range<u64>) -> BoxFuture<'_, parquet::errors::Result<Bytes>> {
+ self.bytes_scanned.add((range.end - range.start) as usize);
+ self.inner.get_bytes(range)
+ }
+
+ fn get_byte_ranges(
Review Comment:
Do we have to implement this?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Experiments with `native_datafusion` scan optimizations [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #3755:
URL: https://github.com/apache/datafusion-comet/pull/3755#discussion_r2971752181
##########
native/core/src/parquet/schema_adapter.rs:
##########
@@ -111,35 +112,41 @@ impl PhysicalExprAdapterFactory for SparkPhysicalExprAdapterFactory {
// to the original physical names. This is necessary because downstream code
// (reassign_expr_columns) looks up columns by name in the actual stream
// schema, which uses the original physical file column names.
- let (adapted_physical_schema, logical_to_physical_names) =
- if !self.parquet_options.case_sensitive {
- let logical_to_physical: HashMap<String, String> = logical_file_schema
- .fields()
- .iter()
- .filter_map(|logical_field| {
- physical_file_schema
- .fields()
- .iter()
- .find(|pf| {
- pf.name().to_lowercase() == logical_field.name().to_lowercase()
- && pf.name() != logical_field.name()
- })
- .map(|pf| (logical_field.name().clone(), pf.name().clone()))
- })
- .collect();
- let remapped =
- remap_physical_schema_names(&logical_file_schema, &physical_file_schema);
- (
- remapped,
- if logical_to_physical.is_empty() {
- None
- } else {
- Some(logical_to_physical)
- },
- )
- } else {
- (Arc::clone(&physical_file_schema), None)
- };
+ let (adapted_physical_schema, logical_to_physical_names) = if !self
+ .parquet_options
+ .case_sensitive
+ {
+ // Pre-compute lowercased physical field names to avoid repeated
+ // to_lowercase() calls in the O(n*m) matching loop.
Review Comment:
Is there any chance we could use `eq_ignore_ascii_case` to avoid allocating the lower case strings?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: cache Parquet metadata in `native_datafusion` scan [datafusion-comet]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on code in PR #3755:
URL: https://github.com/apache/datafusion-comet/pull/3755#discussion_r2975425081
##########
native/core/src/parquet/parquet_read_cached_factory.rs:
##########
@@ -0,0 +1,149 @@
+// 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.
+
+//! A `ParquetFileReaderFactory` that caches parquet footer metadata across
+//! partitions within a single scan. When multiple Spark partitions read from
+//! the same parquet file (different row group ranges), this avoids redundant
+//! footer reads and parsing.
+//!
+//! The cache is scoped to the factory instance (one per scan), not global,
+//! so it does not persist across queries.
+//!
+//! Uses `tokio::sync::OnceCell` per file path so that concurrent partitions
+//! wait for the first reader to load the footer rather than all racing.
+
+use bytes::Bytes;
+use datafusion::common::Result as DataFusionResult;
+use datafusion::datasource::physical_plan::parquet::ParquetFileReaderFactory;
+use datafusion::physical_plan::metrics::{ExecutionPlanMetricsSet, MetricBuilder};
+use datafusion_datasource::PartitionedFile;
+use futures::future::BoxFuture;
+use futures::FutureExt;
+use object_store::path::Path;
+use object_store::ObjectStore;
+use parquet::arrow::arrow_reader::ArrowReaderOptions;
+use parquet::arrow::async_reader::{AsyncFileReader, ParquetObjectReader};
+use parquet::file::metadata::ParquetMetaData;
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::ops::Range;
+use std::sync::{Arc, Mutex};
+use tokio::sync::OnceCell;
+
+type MetadataCell = Arc<OnceCell<Arc<ParquetMetaData>>>;
+
+/// A `ParquetFileReaderFactory` that caches footer metadata by file path.
+/// The cache is scoped to this factory instance (shared across partitions
+/// within a single scan via Arc), not global.
+#[derive(Debug)]
+pub struct CachingParquetReaderFactory {
+ store: Arc<dyn ObjectStore>,
+ cache: Arc<Mutex<HashMap<Path, MetadataCell>>>,
Review Comment:
We should also consider if at some point it should be an LRU with a max capacity, but hte current life cycle should mostly manage that.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Experiments with `native_datafusion` scan optimizations [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #3755:
URL: https://github.com/apache/datafusion-comet/pull/3755#issuecomment-4106696339
Not sure why `partitioned table is cached when partition pruning is true *** FAILED *** (1 second, 574 milliseconds)` 👀
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: cache Parquet metadata in `native_datafusion` scan [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #3755:
URL: https://github.com/apache/datafusion-comet/pull/3755#issuecomment-4111643557
Thanks @andygrove and @mbutrovich in the followup PR I'll try to use caching factory on other context object and alsoinclude metrics
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: `native_datafusion` scan optimizations part1 [datafusion-comet]
Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on code in PR #3755:
URL: https://github.com/apache/datafusion-comet/pull/3755#discussion_r2975199690
##########
native/core/src/parquet/parquet_read_cached_factory.rs:
##########
@@ -0,0 +1,149 @@
+// 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.
+
+//! A `ParquetFileReaderFactory` that caches parquet footer metadata across
+//! partitions within a single scan. When multiple Spark partitions read from
+//! the same parquet file (different row group ranges), this avoids redundant
+//! footer reads and parsing.
+//!
+//! The cache is scoped to the factory instance (one per scan), not global,
+//! so it does not persist across queries.
+//!
+//! Uses `tokio::sync::OnceCell` per file path so that concurrent partitions
+//! wait for the first reader to load the footer rather than all racing.
+
+use bytes::Bytes;
+use datafusion::common::Result as DataFusionResult;
+use datafusion::datasource::physical_plan::parquet::ParquetFileReaderFactory;
+use datafusion::physical_plan::metrics::{ExecutionPlanMetricsSet, MetricBuilder};
+use datafusion_datasource::PartitionedFile;
+use futures::future::BoxFuture;
+use futures::FutureExt;
+use object_store::path::Path;
+use object_store::ObjectStore;
+use parquet::arrow::arrow_reader::ArrowReaderOptions;
+use parquet::arrow::async_reader::{AsyncFileReader, ParquetObjectReader};
+use parquet::file::metadata::ParquetMetaData;
+use std::collections::HashMap;
+use std::fmt::Debug;
+use std::ops::Range;
+use std::sync::{Arc, Mutex};
+use tokio::sync::OnceCell;
+
+type MetadataCell = Arc<OnceCell<Arc<ParquetMetaData>>>;
+
+/// A `ParquetFileReaderFactory` that caches footer metadata by file path.
+/// The cache is scoped to this factory instance (shared across partitions
+/// within a single scan via Arc), not global.
+#[derive(Debug)]
+pub struct CachingParquetReaderFactory {
+ store: Arc<dyn ObjectStore>,
+ cache: Arc<Mutex<HashMap<Path, MetadataCell>>>,
Review Comment:
Long term we should consider adding metrics and/or logging to report sizes of caches like this. It will help us debug possible future edge cases in production without needing complex profilers.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Experiments with `native_datafusion` scan optimizations [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #3755:
URL: https://github.com/apache/datafusion-comet/pull/3755#discussion_r2971755112
##########
native/core/src/parquet/schema_adapter.rs:
##########
@@ -111,35 +112,41 @@ impl PhysicalExprAdapterFactory for SparkPhysicalExprAdapterFactory {
// to the original physical names. This is necessary because downstream code
// (reassign_expr_columns) looks up columns by name in the actual stream
// schema, which uses the original physical file column names.
- let (adapted_physical_schema, logical_to_physical_names) =
- if !self.parquet_options.case_sensitive {
- let logical_to_physical: HashMap<String, String> = logical_file_schema
- .fields()
- .iter()
- .filter_map(|logical_field| {
- physical_file_schema
- .fields()
- .iter()
- .find(|pf| {
- pf.name().to_lowercase() == logical_field.name().to_lowercase()
- && pf.name() != logical_field.name()
- })
- .map(|pf| (logical_field.name().clone(), pf.name().clone()))
- })
- .collect();
- let remapped =
- remap_physical_schema_names(&logical_file_schema, &physical_file_schema);
- (
- remapped,
- if logical_to_physical.is_empty() {
- None
- } else {
- Some(logical_to_physical)
- },
- )
- } else {
- (Arc::clone(&physical_file_schema), None)
- };
+ let (adapted_physical_schema, logical_to_physical_names) = if !self
+ .parquet_options
+ .case_sensitive
+ {
+ // Pre-compute lowercased physical field names to avoid repeated
+ // to_lowercase() calls in the O(n*m) matching loop.
Review Comment:
thats good point
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: fix allocations in schema adapter for `native_datafusion` scan [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead merged PR #3755:
URL: https://github.com/apache/datafusion-comet/pull/3755
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org