jayzhan211 opened a new pull request, #25541: URL: https://github.com/apache/datafusion/pull/25541
## Which issue does this PR close? - N/A — small refactor, no issue filed. ## Rationale for this change `multi_group_by` keeps the set of types `GroupValuesColumn` supports in two places: - `make_group_column`, the factory that builds a `GroupColumn` for a field, and - `group_column_supported_type`, a hand-written allow-list that `supported_schema` uses to choose between `GroupValuesColumn` and the `GroupValuesRows` fallback. The two must accept exactly the same types: if the allow-list accepts a type the factory rejects, the planner picks `GroupValuesColumn` and the query then fails with `not_impl_err` when the columns are built. Today that is enforced by "keep the two in lockstep" comments on both sides (valid `Time32`/`Time64` units, non-negative `FixedSizeBinary` width, the nested/row-backed fallback, recursion into `List` and `Dictionary`) and by a pinning test over a sample of types. Every new specialization has to be added twice. One case had already drifted: a `Dictionary` with a non-integer key type is accepted by the allow-list but rejected by the factory. It is not a valid Arrow type, so it is not reachable in practice, but it shows the shape of the problem. ## What changes are included in this PR? - `group_column_supported_type` now asks the factory: `make_group_column(..).is_ok()`. The 50-line allow-list and the lockstep comments are removed, so the two cannot disagree. Building a column only creates empty buffers, and this runs once per stream, not per batch. - The pinning test is kept as the specification of which types take the column path and which fall back to rows; its doc comment is updated to say so. It gains the drifted `Dictionary(Utf8, Int32)` case, which fails on `main` (`expected group_column_supported_type=false`). No behaviour change for any valid Arrow type. ## What is the testing strategy for this PR? Existing tests: `group_column_supported_type_matches_make_group_column` (supported and intentionally unsupported types, including nested and row-backed cases), the `supported_schema_*` tests and the rest of the `multi_group_by` tests. ## Are there any user-facing changes? No. -- 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]
