dwsmith1983 opened a new pull request, #6042: URL: https://github.com/apache/datafusion-comet/pull/6042
## Which issue does this PR close? Closes #5931. ## Rationale for this change `regr_r2` returns `NULL` under Comet where Spark 3.5.3 returns `1.0` when the independent variable is constant. Comet's serde gates the degenerate-case swap from SPARK-55969 on the Spark minor version, but that change shipped in patch releases: 3.5.9, 4.0.3, 4.1.2 and 4.2.0. Every earlier patch of 3.5, 4.0 and 4.1 keeps the old cases, so Comet answers the swapped way on 3.5.0 through 3.5.8, 4.0.0 through 4.0.2, and 4.1.0 and 4.1.1. CI builds against 3.5.9, 4.0.4 and 4.1.3, which is why the existing fixture never noticed. Anyone running Comet against 3.5.0 through 3.5.8, 4.0.0 through 4.0.2, or 4.1.0 and 4.1.1 sees `regr_r2` return `NULL` for a constant `x` and `1.0` for a constant `y` where Spark returns the opposite. Every CI profile (3.4.3, 3.5.9, 4.0.4, 4.1.3, 4.2.0) sits on the post-swap side, so the `regr.sql` fixture, which compares Comet with the running Spark, could not catch it. The sibling flag has the same shape. SPARK-48719 made `regr_slope` and `regr_intercept` count `VariancePop(x)` only over rows where both arguments are non-null, and it shipped in 3.5.2 and 4.0.0, so Comet also applied it on 3.5.0 and 3.5.1, where Spark still counts every row with a non-null `x`. ## What changes are included in this PR? - A `RegrSparkVersions` object in the aggregate serde with two predicates over the running Spark version: `slopeFiltersVarByPairNulls` is true from 3.5.2 and 4.0.0, and `r2DegenerateCasesSwapped` is true from 3.5.9, 4.0.3, 4.1.2 and 4.2.0. The leading `major.minor.patch` is parsed locally because Spark's `VersionUtils` is private to its packages; a suffix such as `-SNAPSHOT` is ignored and an unparsable version falls back to the previous minor-version rule. - `CometRegrBase.convertRegr` sets the two protobuf flags from those predicates instead of `isSpark35Plus`. - The native accumulator is unchanged; its comments and the `regr.sql` fixture comments now name the exact releases, replacing a note that placed the swap in 4.1.0. ## How are these changes tested? A new test in `CometAggregateSuite` checks both predicates against a table of Spark versions: the release just before and just after each change on every affected line, a hypothetical 3.6.0 and 5.0.0, a vendor suffix, a patchless `3.5`, and an unparsable string that pins the fallback. It fails when the predicates are replaced with the previous `>= "3.5"` rule (`slope pair-null filtering for Spark 3.5.0 should be false`). The boundaries were verified against the tagged Spark sources for 3.4.3, 3.5.1, 3.5.2, 3.5.8, 3.5.9, 4.0.1, 4.0.2, 4.0.3, 4.1.1, 4.1.2 and 4.2.0. The existing `regr.sql` fixture still compares Comet with Spark for both degenerate cases and passes on the CI versions, whose answers do not change. Run locally on Spark 3.5 with JDK 17: `CometAggregateSuite`, spotless, the semantic scalafix check, `cargo fmt` and the native `regr` tests. -- 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]
