andygrove opened a new pull request, #7248: URL: https://github.com/apache/datafusion-comet/pull/7248
## Which issue does this PR close? No tracking issue. This came from a production fallback warning on a query that filters on `flatten(flatten(array_col))[0]` over a column of type `array<array<array<binary>>>`. ## Rationale for this change The query fell back to Spark for the aggregate that consumes the filter, with this explain output: ``` HashAggregate [COMET: Comet aggregate that merges intermediate buffers requires a Comet child aggregate ...] +- HashAggregate [COMET: Unsupported aggregate expression(s), data type not supported: ArrayType(ArrayType(BinaryType,true),true)] ``` Two things were wrong: 1. `CometFlatten` used the shared array type check, which rejects `BinaryType`. That check exists for functions that compare elements (`array_except`, `array_remove`), but `flatten` only rearranges offsets and never reads the values, so there is no reason to refuse binary elements. 2. The reason did not say which function rejected the type. Reasons are recorded on the expression that rejects the input, but `rollUpFallbackReasons` lifts them onto the operator as a flat set of strings, so the expression is no longer visible in the explain output. ## What changes are included in this PR? Two commits: - `CometFlatten` accepts binary elements, including nested in arrays. - `rollUpFallbackReasons` prefixes the name of the expression that recorded each reason (`flatten: data type not supported: ...`). It leaves the reason unchanged for literals, aliases and attributes (their names add nothing), for aggregate expressions it uses the function name, and it skips reasons that already mention the expression name or name a `spark.comet.` config key (the key names the expression to enable, which need not be the one that recorded the reason). ## How are these changes tested? - `flatten.sql` gains binary cases: `flatten` over `array<array<binary>>` with empty, null and empty-binary values, `count` and `count(DISTINCT ...)` over it, and the query shape from the report (nested `flatten` indexed with `[0]` in a filter, `COUNT(1)` and `COUNT(DISTINCT)`, grouped and ungrouped). Plain `query` blocks assert the plan runs natively. - `CometExecRuleSuite` gains a test for the three prefixing rules. - Run locally on the default Spark 4.1 profile: `CometExecRuleSuite`, `CometSqlFileTestSuite` (`flatten` and `array/`), `CometCollationSuite`, `CometScalarFunctionSuite`, `CometCastCollatedStringSuite`, `CometCodegenSuite`. - The remaining suites that assert on fallback reasons are still running; I will update this section with the results. -- 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]
