snmvaughan commented on issue #7241: URL: https://github.com/apache/datafusion-comet/issues/7241#issuecomment-6751803848
Thanks for the detailed review, @andygrove. I've opened #7577, and it follows all of it. **Inventory.** `CometPercentile.keepsSignedZeroKeys` is in. `CometMode.keysNaNsTogether` moves in a follow-up once #7115 merges, so this one stays with what is on main. `isSparkVersionAtLeast355` stays where it is, and I've added a line to the issue saying so. **Parser rule.** All of the checks use `Utils.majorMinorPatchVersion` and count a version it can't parse as unfixed. `RegrSparkVersions`, with its regex and the `>= "3.5"` fallback, is gone. The strings that change are the ones that parsed differently: `v3.5.9` and `4.0.5_vendor` are now unfixed for the two regression checks, and a bare `4` reads as `4.0.0`. The PR says so, and the two `CometAggregateSuite` rows moved to `SparkFixesSuite`. **Shape.** Each fix has its own predicate and its own table in `SparkFixes`, including SPARK-58821 and SPARK-60118, which no longer share `fixedIn`. The tables moved to `SparkFixesSuite`, the three docs name `SparkFixes`, and I compared the old and new predicates over 2,390 version strings: the answers are the same apart from the strings above. #7404 does the parser half and edits `RegrSparkVersions.majorMinorPatch`, which the PR deletes, so whichever merges second has a one-hunk conflict in `aggregates.scala`. -- 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]
