andygrove opened a new issue, #26309: URL: https://github.com/apache/datafusion/issues/26309
### Is your feature request related to a problem or challenge? A review of complexity and duplication across DataFusion's Rust code at commit 0d79252f61bd turned up a set of hotspots: functions that are very large and change often, and code copied in several places where a fix to one copy can miss the others. This epic tracks the follow-up issues so they can be picked up independently. All sub-issues are labelled `tech debt` and `help wanted`; the smaller ones are also `good first issue`. Most refactors are meant to be done incrementally, as behaviour-preserving PRs that rely on the existing tests. ### Describe the solution you'd like **Bugs found along the way** - [ ] #26295 Unparser silently drops CASE operand and ELSE branch when they fail to unparse - [ ] #26296 Struct union type coercion takes field nullability from the wrong field when field order differs - [ ] #26297 gcd benchmarks pass `number_rows: 0` for 100k-row inputs **Large, frequently changed functions** - [ ] #26298 Split `Unparser::select_to_sql_recursively` into per-plan-node methods (896 lines, cognitive complexity 272) - [ ] #26299 Break up `ensure_distribution_with_stats` and `enforce_distribution_relationships` in `EnforceDistribution` - [ ] #26300 Move large `LogicalPlan` arms out of `map_logical_node_to_physical` (1,203 lines, 80 commits in 12 months) - [ ] #26301 Move large `Statement` arms out of `sql_statement_to_plan_with_context_impl` (1,307 lines) - [ ] #26305 Split `PushDownFilter::rewrite` into per-plan-node pushdown helpers **Duplicated logic** - [ ] #26302 Deduplicate copy-pasted rule families in `Simplifier::f_up` - [ ] #26303 Share a single pad kernel between `lpad` and `rpad` - [ ] #26304 Deduplicate `Concat` string and binary builders in datafusion-functions **Good first issues** - [ ] #26306 Use the shared `aggr_test_schema` in CSV scan tests instead of a local copy - [ ] #26307 Deduplicate bucket computation in `width_bucket_interval_mdn_exact` - [ ] #26308 Merge duplicated wildcard arms in `project_with_validation` ### Describe alternatives you've considered Some findings were deliberately left out of this list: - `collect_left_input` in the hash join (14 parameters, ~450 lines) is also a hotspot, but #24768 (spilling hash join) will rework it, so a separate refactor would conflict. It is better addressed as part of that work. - `try_into_logical_plan` (proto), `get_valid_types`, `window_equivalence_properties` and decorrelate's `PullUpCorrelatedExpr::f_up` are similar but lower-priority candidates. - Test-only duplication (statistics literals, aggregate test fixtures, proto round-trip scaffolding, the `assert_optimized_plan_equal!` macro redefined in at least 8 optimizer test modules) is real but low risk. ### Additional context Line references in the sub-issues 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]
