0lai0 opened a new pull request, #4157:
URL: https://github.com/apache/datafusion-comet/pull/4157

   ## Which issue does this PR close?
   
   Closes #4123
   
   ## Rationale for this change
   
   Spark 4.1.1 SQL coverage exposed a case where Comet allows native 
aggregation for grouping keys that contain nested `MapType` (for example 
`StructType(MapType(...))`).  
   Although planning succeeds, downstream native sort/row-format handling does 
not support this shape, so the query fails at execution time instead of falling 
back to Spark during planning.
   
   This PR makes aggregate planning reject map-containing grouping key types so 
these queries reliably fall back to Spark.
   
   ## What changes are included in this PR?
   
   - add a recursive `containsMapType` helper in `CometBaseAggregate`
   - make aggregate planning fall back when any grouping expression contains 
`MapType` directly or nested in `StructType`/`ArrayType`
   - add regression test in `CometAggregateSuite` for grouping on 
`Struct(Map(...))`
   - add regression test in `CometAggregateSuite` for grouping on 
`Array(Map(...))`
   
   ## How are these changes tested?
   
   - `./mvnw test -Pspark-4.1 -Dtest=none 
-Dsuites="org.apache.comet.exec.CometAggregateSuite grouping on struct 
containing map should fallback to Spark"`
   - `./mvnw test -Pspark-4.1 -Dtest=none 
-Dsuites="org.apache.comet.exec.CometAggregateSuite group by array of map falls 
back to Spark (issue #4123)"`
   - `./mvnw test -Pspark-4.1 -Dtest=none 
-Dsuites="org.apache.comet.exec.CometAggregateSuite"`


-- 
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]

Reply via email to