emanuele-em opened a new pull request, #26374: URL: https://github.com/apache/datafusion/pull/26374
## Which issue does this PR close? Part of #26302. This PR covers the boolean-literal comparison family only and does not close the full issue. ## Rationale for this change Keep the existing boolean predicate simplifications consistent when maintaining equality and inequality rules. The issue requests small, behavior-preserving changes, one rule family at a time. The issue discussion also contains another contributor's plan for the same Eq/NotEq family: https://github.com/apache/datafusion/issues/26302#issuecomment-6192749227. No matching open PR was found when preparing this draft. Coordination on this overlap is pending; this draft makes the completed refactor available for that discussion. ## What changes are included in this PR? - Share the boolean-literal Eq/NotEq rewrites through a private helper while keeping the flat guarded match structure and the existing type checks. - Expand the existing comparison tests to cover both operand orders, nullable and non-nullable boolean columns, boolean NULLs, and nonboolean operands. ## What is the testing strategy for this PR? The existing Rust comparison tests disable canonicalization so both literal positions reach the simplifier directly. They also check the nonboolean type guards on raw expressions before type coercion. These are characterization tests for a refactor and passed before and after the production change. Completed validation: - `cargo test --profile ci -p datafusion-optimizer`: 941 unit tests, 29 integration tests, and 5 doctests passed; 1 doctest ignored. - Ran the built SQL logic test binary with the `simplify` filter: all 4 matching files passed. - `cargo fmt --all` and the CI Clippy configuration passed. TOML formatting, license headers, spelling, and Markdown formatting also passed. - Compared the existing `simplify_function_over_case` simplifier benchmark against unchanged main at cf4869cd834dfa2756a257cbd5b66f8a5f2ec7a6. The three cases showed no statistically significant regression. This benchmark exercises the general simplifier and does not directly measure the boolean comparison family; no speedup is claimed. Validation gaps: - `cargo clippy --all-targets --all-features -- -D warnings` reported 3 `mutable_key_type` errors in unchanged `push_down_filter.rs`. A focused all-features Clippy run reproduced the same errors on unchanged main. - The extended workspace run failed. Ordinary test binaries recorded 12,670 passes, 1 catalog lookup failure, and 8 ignored tests. The full SQL logic runner also failed with fixture parsing and path errors. Relocating the same binary exposed its compiled-in path normalization, so the full SQL suite cannot be reported as passing. - The complete `./dev/rust_lint.sh` suite did not finish; it was stopped at documentation generation after the checks listed above passed. - The default `cargo test -p datafusion` and `cargo test -p datafusion-cli` commands have not been run separately. The extended workspace command excluded the CLI. @alamb, could you help trigger CI for this first contribution? This PR is a draft with the validation gaps above still open. ## Are there any user-facing changes? No intended changes to SQL results or public Rust APIs. This is an internal refactor of existing simplification rules. -- 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]
