This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-25664-21a3215b66a8620cb932899de419e6b43d1848e6 in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit e772e2053162c5cf4ac60c22470f123c2d9f16a3 Author: Bill Easton <[email protected]> AuthorDate: Thu Sep 24 14:00:50 2026 +0000 Avoid formatting errors for expected missing physical columns (#25664) ## Which issue does this PR close? Closes #25663. ## Rationale for this change Schema adaptation can legitimately encounter columns absent from an older physical schema. A missing-column lookup currently formats an error listing every physical field, then discards it. This adds avoidable latency as schemas widen. ## What changes are included in this PR? Use the non-error-producing field lookup in the physical expression adapter. Add a Criterion benchmark for present and missing columns in 16- and 128-field schemas. ## What is the testing strategy for this PR? The existing adapter tests cover missing nullable and non-nullable columns and present columns. The full extended workspace test command passed locally, as did formatting and strict package-level Clippy. Full-workspace Clippy currently reports two warnings in unchanged `datafusion/physical-plan/benches/multi_group_by.rs`. On Apple Silicon, the missing-column benchmark changed from 537 to 256 ns for 16 fields and 1.83 us to 302 ns for 128 fields. Present-column timings were within run-to-run variation. These are microbenchmarks, not end-to-end query measurements. ## Are there any user-facing changes? No behavior or API changes; this reduces work for expected missing columns. --- Cargo.lock | 1 + datafusion/physical-expr-adapter/Cargo.toml | 5 +++ .../benches/schema_rewrite.rs | 51 ++++++++++++++++++++++ .../physical-expr-adapter/src/schema_rewriter.rs | 3 +- 4 files changed, 59 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 06c12a9e07..d63db0b271 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2472,6 +2472,7 @@ name = "datafusion-physical-expr-adapter" version = "55.1.0" dependencies = [ "arrow", + "criterion", "datafusion-common", "datafusion-expr", "datafusion-functions", diff --git a/datafusion/physical-expr-adapter/Cargo.toml b/datafusion/physical-expr-adapter/Cargo.toml index 453c8bdaac..fef92203d9 100644 --- a/datafusion/physical-expr-adapter/Cargo.toml +++ b/datafusion/physical-expr-adapter/Cargo.toml @@ -28,3 +28,8 @@ itertools = { workspace = true } workspace = true [dev-dependencies] +criterion = { workspace = true } + +[[bench]] +name = "schema_rewrite" +harness = false diff --git a/datafusion/physical-expr-adapter/benches/schema_rewrite.rs b/datafusion/physical-expr-adapter/benches/schema_rewrite.rs new file mode 100644 index 0000000000..29fa0b8094 --- /dev/null +++ b/datafusion/physical-expr-adapter/benches/schema_rewrite.rs @@ -0,0 +1,51 @@ +// 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. + +use std::hint::black_box; +use std::sync::Arc; + +use arrow::datatypes::{DataType, Field, Schema}; +use criterion::{Criterion, criterion_group, criterion_main}; +use datafusion_physical_expr::expressions::Column; +use datafusion_physical_expr_adapter::{DefaultPhysicalExprAdapter, PhysicalExprAdapter}; + +fn bench_schema_rewrite(c: &mut Criterion) { + for field_count in [16, 128] { + let physical_fields: Vec<_> = (0..field_count) + .map(|index| Field::new(format!("field_{index}"), DataType::Utf8, true)) + .collect(); + let mut logical_fields = physical_fields.clone(); + logical_fields.push(Field::new("optional_new_field", DataType::Utf8, true)); + + let adapter = DefaultPhysicalExprAdapter::new( + Arc::new(Schema::new(logical_fields)), + Arc::new(Schema::new(physical_fields)), + ); + let missing = Arc::new(Column::new("optional_new_field", field_count)); + let present = Arc::new(Column::new("field_0", 0)); + + c.bench_function(&format!("schema_rewrite/missing/{field_count}"), |b| { + b.iter(|| black_box(adapter.rewrite(black_box(missing.clone())).unwrap())) + }); + c.bench_function(&format!("schema_rewrite/present/{field_count}"), |b| { + b.iter(|| black_box(adapter.rewrite(black_box(present.clone())).unwrap())) + }); + } +} + +criterion_group!(benches, bench_schema_rewrite); +criterion_main!(benches); diff --git a/datafusion/physical-expr-adapter/src/schema_rewriter.rs b/datafusion/physical-expr-adapter/src/schema_rewriter.rs index b6364fcfe9..56be58f4dc 100644 --- a/datafusion/physical-expr-adapter/src/schema_rewriter.rs +++ b/datafusion/physical-expr-adapter/src/schema_rewriter.rs @@ -772,7 +772,8 @@ impl DefaultPhysicalExprAdapterRewriter { // resolving by name ensures we match the correct physical slot. Once we know the // proper index we rebuild the `Column` with `new_with_schema` so callers can rely // on `column.index()` later without having to re-query the schema. - let Ok(physical_column_index) = self.physical_file_schema.index_of(column.name()) + let Some((physical_column_index, _)) = + self.physical_file_schema.fields().find(column.name()) else { return Ok(None); }; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
