crm26 commented on PR #21363:
URL: https://github.com/apache/datafusion/pull/21363#issuecomment-4208890293
@neilconway You were right — I got two of the three wrong in the last push.
Apologies for the noise.
Pushed fixes in `93341e033`:
1. **"Optimization:" prefix** — I put it on the alias preservation comment
last time instead of the skip check you actually pointed at. Moved it to the
correct line (`// Optimization: skip if no predicate subqueries in any
projection expression` at the top of the Projection match arm) and cleaned up
the stray prefix on the alias comment.
2. **Cloning / early-return simplification** — I skipped this in the last
push entirely. Now mirrors `ScalarSubqueryToJoin`:
- `let mut cur_input = projection.input.as_ref().clone();`
- Dropped `original_input` and `Projection::try_new_with_schema`
- Bail-out just returns
`Ok(Transformed::no(LogicalPlan::Projection(projection)))` directly
- Removed the now-unused `Projection` import
- The one structural difference from `ScalarSubqueryToJoin`: I clone
`projection.expr` upfront (`let projection_exprs = projection.expr.clone();`)
so the loop can consume owned expressions without borrowing `projection`, which
lets the bail-out move it. A comment at the clone site explains why. Happy to
restructure if you'd prefer a different approach.
3. **Per-expr bail-out** and **mixed decorrelatable+non-decorrelatable
test** from the previous push are unchanged.
Validation I ran locally this time (full suite, not just the targeted file):
- `cargo fmt --check` + `cargo clippy -p datafusion-optimizer -- -D
warnings` — clean
- `cargo test -p datafusion-optimizer` — 665 + 26 + 5 pass (59 decorrelate
tests)
- `cargo test --test sqllogictests` — all 422 files pass
- `cargo test --profile ci-optimized --test sqllogictests --features
backtrace -- --include-sqlite` — all 1017 files pass
- `cargo test -p datafusion-cli --features datafusion/extended_tests` — 61 +
7 + 25 pass
Will resolve the review threads in the UI once CI confirms. The "Predicate"
→ "ExistsInSubquery" rename is noted for a followup PR as you suggested.
--
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]
