peter-toth commented on PR #59522: URL: https://github.com/apache/spark/pull/59522#issuecomment-6285700295
Thanks for the PR, @rekbun. I'd rather not make the grouping logic more complex without a concrete case that needs it. Could you share which connector and what table layout you hit this with? The only connector I know of that reports `KeyGroupedPartitioning` through `SupportsReportPartitioning` is Iceberg, and it doesn't seem to have this problem. With `spark.sql.iceberg.planning.preserve-data-grouping` on, it packs the files of each partition into task groups of `read.split.target-size` (128 MB by default). So it reports several splits for a key only when the key holds more data than that, or when adaptive split sizing splits a small scan to use the cluster's parallelism. In both cases one task per split is what we want below a shuffle. If a scan doesn't need its `KeyGroupedPartitioning` but has too many small splits, grouping them by key may not be the best answer either. It brings back the pre-4.2 behavior, and I think the current one is better: - A large key becomes one large task. - It undoes Iceberg's adaptive split sizing. There are other ways to cut the number of tasks: - `coalesce()` or the `COALESCE` hint over the scan works as a workaround today. - A size-based packing could use `HasPartitionStatistics.sizeInBytes()`, like the file sources do with `spark.sql.files.maxPartitionBytes`. - Or another connector could pack its splits the way Iceberg does. -- 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]
