namanjain24-sudo commented on PR #25527: URL: https://github.com/apache/datafusion/pull/25527#issuecomment-6200210546
Breakdown of the 1256 lines: ~600 is tests/fixtures (330 in `roundtrip_logical_plan.rs`, 205 in two JSON test plans, 64 in `aggregation_tests.rs`). Actual logic is `grouping_set.rs` (395 lines, shared by producer and consumer) plus net +42 producer / +23 consumer, so ~460 lines of real code, not 1150. On the expression — yeah, it's a lookup: `CASE WHEN __grouping_id = ids[0] THEN 0 ... END` (and the inverse on the producer side). Can't do this as a plain bit-unpack because Substrait's index is the set's position in the aggregate's own declared list — whatever order the query wrote it in, or `ROLLUP`/`CUBE` expanded it into. That position isn't derivable from `__grouping_id`'s `(ordinal << group_count) | mask` by formula; two aggregates with the same masks can order their sets differently. So recovering the index needs this aggregate's specific set list, which is what the `CASE` is encoding. Open to a cheaper encoding than `Expr::Case` if you've got one in mind, just don't think arithmetic alone gets us there. -- 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]
