⚡ Bolt: optimize logical2physical to avoid Schema deep copy#276
⚡ Bolt: optimize logical2physical to avoid Schema deep copy#276
Conversation
This optimization modifies `logical2physical` to accept `&SchemaRef` instead of `&Schema`. By doing so, it avoids a deep copy of the Arrow schema's fields when converting to `DFSchema`. The existing implementation triggered a deep copy because it only had a reference to the `Schema` and needed an owned `Arc<Schema>` to create a `DFSchema`. Impact: Reduces CPU and memory overhead when creating physical expressions from logical ones, especially in contexts with many expressions or large schemas (e.g. Parquet scan predicates). Measurement: Verified with existing tests and `cargo check`. The optimization addresses a long-standing `TODO` in the codebase. Co-authored-by: Dandandan <163737+Dandandan@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
1 participant
The optimization improves
logical2physicalby accepting&SchemaRef(&Arc<Schema>) instead of&Schema. This allows the function to useArc::clonerather than a deep copy of the schema's fields. I have also updated all call sites in the repository to accommodate this change, ensuring that they either pass existingSchemaRefs or wrap ownedSchemas in anArc. Tests fordatafusion-physical-expr,datafusion-datasource-parquet, anddatafusion-pruninghave been run and passed.PR created automatically by Jules for task 1702757292747917788 started by @Dandandan