[AURON #2499] perf: avoid identity take in GenerateExec - #2500
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is localized and aligns with the existing row-id construction logic; only a minor comment-clarity nit was identified.
Pull request overview
This PR optimizes GenerateExec by skipping an unnecessary Arrow take on generator output columns when outer is false, avoiding redundant array allocations/copies while preserving correctness for outer generation and parent-row duplication.
Changes:
- Reuse
generated_outputs.colsdirectly for non-outer generation to avoid identitytake. - Keep the
takepath foroutergeneration where NULL padding is required.
File summaries
| File | Description |
|---|---|
| native-engine/datafusion-ext-plans/src/generate_exec.rs | Avoids redundant identity take on generated output arrays when outer is false. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Without outer rows, generated_ids is always the identity | ||
| // sequence 0..generated_outputs.len(). Reuse the generated | ||
| // arrays instead of copying them through Arrow take. |
slfan1989
left a comment
There was a problem hiding this comment.
Thanks for this performance improvement!
One process item before merging:
the PR marks generative AI tooling as “Yes”, but the Generated-by field still contains the placeholder.
Please replace it with the actual tool name and version.
LGTM otherwise.
|
@slfan1989 Thanks for the review! I've updated the Generated-by field. |
Which issue does this PR close?
Closes #2499
Rationale for this change
Avoid redundant generated-array copies during non-outer
GenerateExec.What changes are included in this PR?
outeris false.takepath for outer generation.takerequired to duplicate parent rows.Are there any user-facing changes?
No. This is an internal performance improvement.
How was this patch tested?
Existed unit tests passed.
Benchmarks
The benchmark uses 10,000 input rows with 8 values per row. The reported values are averages of two runs.
explode_array_i32posexplode_array_i32explode_map_utf8_i32explode_array_i32_parent_null_gapsWas this patch authored or co-authored using generative AI tooling?
If yes, include:
Generated-by: gpt-5.6ASF guidance: https://www.apache.org/legal/generative-tooling.html