[I] Native regr_* aggregates return wrong results for a constant variable merged from several partitions [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove opened a new issue, #6423:
URL: https://github.com/apache/datafusion-comet/issues/6423

   ### Describe the bug
   
   `regr_slope`, `regr_intercept`, `regr_r2`, `regr_sxx`, `regr_syy` and `regr_sxy` run natively by default since #4775. The bug needs two conditions:
   
   - a group's variable is constant at a value that binary floating point can't represent exactly, such as 0.1
   - the group's rows are merged from two or more partial aggregates
   
   When both hold, Comet returns plausible-looking wrong values where Spark returns NULL or 0.0. In 1.0.0 these aggregates ran in Spark, so this is a regression in 1.1.0. It reproduces on 1.1.0-rc1 and on `main`.
   
   ### Steps to reproduce
   
   ```scala
   // In a suite extending CometTestBase, default configs
   spark.range(0, 6, 1, 2)
     .selectExpr("CAST(id AS DOUBLE) AS y", "0.1D AS x")
     .write
     .parquet(path) // two files, so two scan partitions
   withParquetTable(path, "t") {
     checkSparkAnswer(
       "SELECT regr_slope(y, x), regr_intercept(y, x), regr_r2(y, x), regr_sxx(y, x), " +
         "regr_sxy(y, x), regr_r2(x, y), regr_syy(x, y) FROM t")
   }
   ```
   
   ### Expected behavior
   
   Spark and Comet 1.0.0 return `NULL, NULL, NULL, 0.0, 0.0, 1.0, 0.0`.
   
   Comet 1.1.0-rc1, with `CometHashAggregate` in both the partial and the final stage, returns `2.16e17, -2.16e16, 0.7714, 2.89e-34, 6.2e-17, 0.7714, 2.89e-34` (Spark 4.1 profile).
   
   ### Additional context
   
   The merge in `native/spark-expr/src/agg_funcs/welford.rs` (`variance_merge`, `covariance_merge`) uses a different floating-point operation order from Spark's `CentralMomentAgg` and `Covariance`. The first merge into the zero-initialized final buffer leaves a one-ULP error on the mean. As a result `m2` ends up around 1e-34 instead of exactly 0, and Spark's exact `m2 == 0` checks for the degenerate cases never fire.
   
   The same merge order already makes multi-partition `var_pop` and `stddev_pop` drift on clustered values in 1.0.0. The tests in #4775 use whole-number constants and a tolerance, which hide it.
   
   For 1.1.0 the smallest fix is to mark the `regr_*` aggregates Incompatible, so they fall back to Spark as they did in 1.0.0. Porting Spark's merge order is the full fix, and it would also fix `var_pop` and `stddev_pop`.
   
   Found by the 1.1.0 regression audit (#6399) and tracked in #6402.
   


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Native regr_* aggregates return wrong results for a constant variable merged from several partitions [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove closed issue #6423: Native regr_* aggregates return wrong results for a constant variable merged from several partitions
URL: https://github.com/apache/datafusion-comet/issues/6423


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org