parthchandra commented on PR #6451: URL: https://github.com/apache/datafusion-comet/pull/6451#issuecomment-5919289809
The guard is complete - all five regr serdes that use the buggy covariance merge now fall back, and `regr_count`/`avgx`/`avgy` correctly stay native (they rewrite to Count/Average). Two notes: - **`spark/src/main/scala/org/apache/comet/serde/aggregates.scala:1057`** — worth confirming in the thread that sharing one opt-in key (`RegrReplacement`) for both `regr_sxx` and `regr_syy` is intended: a user who opts into `regr_sxx` silently also opts `regr_syy` into the native path. This falls straight out of Spark planning both as `RegrReplacement`, and the docs already spell it out, so this is a "confirm it's understood" note, not a defect. - **`spark/src/test/scala/org/apache/comet/exec/CometAggregateSuite.scala:1833`** — reading the parquet back can coalesce two small files into one partition under the default `maxPartitionBytes`/`openCostInBytes`, which would trip the `getNumPartitions == 2` assert on some setups rather than reproducing the merge. It fails loudly instead of passing on a wrong premise, so it's safe, but pinning the read so the two-partition layout is guaranteed across Spark versions would make it robust. -- 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]
