Skip to content

Deduplicate copy-pasted rule families in Simplifier::f_up #26302

Open

Description

@andygrove

Is your feature request related to a problem or challenge?

Simplifier::f_up (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 0d79252.

Activity

  1. added
    enhancementNew feature or request
    help wantedExtra attention is needed
    tech debtCode complexity, duplication or structure that makes the codebase harder to change
    on Oct 14, 2026
  2. mohitgurav20 commented on Oct 15, 2026

    @mohitgurav20
    Contributor

    take

  3. mohitgurav20 commented on Oct 15, 2026

    @mohitgurav20
    Contributor

    I can take this on. Doing it in small, behavior-preserving increments makes total sense to keep reviews focused and safe.

    I'd propose starting with a first PR that tackles the boolean-literal symmetry for Eq / NotEq (collapsing the mirrored left/right arms into a helper) along with fixing the BitwiseShiftRight comment typo, before moving to the absorption/idempotence helpers in follow-ups.

    take

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requesthelp wantedExtra attention is neededoptimizerOptimizer rulestech debtCode complexity, duplication or structure that makes the codebase harder to change

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions