andygrove opened a new issue, #26302: URL: https://github.com/apache/datafusion/issues/26302
### Is your feature request related to a problem or challenge? [`Simplifier::f_up` (expr_simplifier.rs#L862-L2168)](https://github.com/apache/datafusion/blob/0d79252f61bd8777990b2749c216ffd07079c30d/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs#L862-L2168) is a single 972-line match with cyclomatic complexity 267, holding every rewrite rule as a guarded arm. The file has had 42 commits in the last 12 months. A flat list of guarded rules is a deliberate, readable DataFusion idiom, and this issue does not propose replacing it. The problem is that near-identical rule families are copy-pasted per operator: - The absorption and idempotence rules for `OR` (about L1021-L1043), `AND` (about L1111-L1133), `BitwiseAnd` (about L1270-L1300), `BitwiseOr` (about L1320-L1350) and `BitwiseXor` (about L1370-L1400) differ only in the operator and an extra is-not-null guard. - The left/right boolean-literal arms for `Eq` (about L897-L925) and `NotEq` (about L947-L975) mirror each other. - The section at L1416 is commented `// Rules for BitwiseShiftRight` but contains the `BitwiseShiftLeft` rules; L1405 has the same heading. A fix to one member of a family has to be repeated by hand for the others, and the size of the function makes individual rules hard to find. ### Describe the solution you'd like - Factor each rule family into a generic helper parameterised by `Operator` (e.g. one absorption/idempotence helper shared by AND/OR and the bitwise variants), and fold the mirrored left/right literal arms into a helper taking `(literal_side, other_side)`. - Optionally dispatch on operator first into per-family functions (`simplify_or`, `simplify_and`, `simplify_bitwise`, `simplify_case`, ...) returning `Transformed<Expr>`. - Fix the duplicated `BitwiseShiftRight` comment. Do it one family at a time, with behaviour-preserving PRs relying on the existing simplifier tests. ### Describe alternatives you've considered Only fix the comment and leave the structure, at the cost of continued duplication between rule families. ### Additional context Line references are pinned to commit 0d79252f61bd. -- 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]
