cetra3 commented on code in PR #26630:
URL: https://github.com/apache/datafusion/pull/26630#discussion_r4384297643
##########
datafusion/common/src/dfschema.rs:
##########
@@ -118,6 +120,13 @@ pub struct DFSchema {
field_qualifiers: Vec<Option<TableReference>>,
/// Stores functional dependencies in the schema.
functional_dependencies: FunctionalDependencies,
+ /// Lazily built lookup from field name to positions in `inner`, only used
+ /// for schemas with more than [`LINEAR_NAME_SCAN_MAX`] fields.
+ ///
+ /// Derived from the field names alone, so it is shared by clones and by
+ /// schemas that only change qualifiers, and must be reset whenever `inner`
+ /// changes. It takes no part in equality, hashing or `Debug` output.
+ name_index: OnceLock<Arc<NameIndex>>,
Review Comment:
I benchmarked an eager variant, which builds the index in the constructors
once a schema has more than 8 fields. `sql_planner` against `main`, both
variants from the same run:
| Benchmark | Eager | Lazy (this PR) |
|---|---|---|
| `logical_select_one_from_700` | +7.8% | +0.5% |
| `logical_union_8_branches` | +6.9% | +1.3% |
| `physical_join_distinct` | +5.5% | +0.2% |
| `*_select_all_from_1000` | −67% | −67% |
Most wide schemas built during planning are never looked up by name, so
eager pays for indexes that are never used. I've kept lazy.
You're right that the lint change is user-facing, so I've added it to the
user-facing section of the PR body: downstream crates that use `Expr` or
`LogicalPlan` as hash keys and lint with `-D warnings` need
`datafusion_common::DFSchema` in `ignore-interior-mutability`.
--
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]
