[VL] Support Spark legacy statistical aggregation function behavior by NEUpanning · Pull Request #9181 · apache/gluten

NEUpanning · 2025-03-31T06:43:27Z

What changes were proposed in this pull request?

To align with Spark, facebookincubator/velox#12566 introduced spark.legacy_statistical_aggregate configuration, which controls whether NULL or NaN is returned when dividing by zero. This PR enables this config if spark.sql.legacy.statisticalAggregate is set to true.

How was this patch tested?

integration tests

github-actions · 2025-03-31T06:43:44Z

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

Other pull requests

github-actions · 2025-03-31T06:44:00Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-04-01T03:54:20Z

Run Gluten Clickhouse CI on x86

NEUpanning · 2025-04-01T07:05:23Z

@rui-mo Could you help to review this PR? Thanks.

jinchengchenghh · 2025-04-01T16:02:30Z

shims/common/src/main/scala/org/apache/gluten/config/GlutenConfig.scala

      (SQLConf.CASE_SENSITIVE.key, SQLConf.CASE_SENSITIVE.defaultValueString),
      (SQLConf.IGNORE_MISSING_FILES.key, SQLConf.IGNORE_MISSING_FILES.defaultValueString),
      (SQLConf.LEGACY_TIME_PARSER_POLICY.key, SQLConf.LEGACY_TIME_PARSER_POLICY.defaultValueString),
+      (


The default value is same with native backend value std::to_string(veloxCfg_->get<bool>(kSparkLegacyStatisticalAggregate, false));. Add the key in L470 is enough.

I think using the default value that aligns with Spark would be great. Maybe delete the default value in std::to_string(veloxCfg_->get<bool>(kSparkLegacyStatisticalAggregate, false));

If user doesn't set the config, I think it's better we don't set it too.

If user doesn't set this config, the SQLConf.LEGACY_STATISTICAL_AGGREGATE.defaultValueString will be used rather than false, although it is also false now.

jinchengchenghh · 2025-04-01T16:04:55Z

shims/common/src/main/scala/org/apache/gluten/config/GlutenConfig.scala

@@ -509,6 +510,9 @@ object GlutenConfig {
      (SQLConf.CASE_SENSITIVE.key, SQLConf.CASE_SENSITIVE.defaultValueString),
      (SQLConf.IGNORE_MISSING_FILES.key, SQLConf.IGNORE_MISSING_FILES.defaultValueString),
      (SQLConf.LEGACY_TIME_PARSER_POLICY.key, SQLConf.LEGACY_TIME_PARSER_POLICY.defaultValueString),


So as LEGACY_TIME_PARSER_POLICY.

rui-mo

Better to add test in Gluten to ensure this config could take effect. Thanks.

github-actions · 2025-04-03T09:28:48Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-04-07T03:54:39Z

Run Gluten Clickhouse CI on x86

.../spark33/src/test/scala/org/apache/spark/sql/execution/GlutenSQLAggregateFunctionSuite.scala

github-actions · 2025-04-07T08:53:14Z

Run Gluten Clickhouse CI on x86

NEUpanning · 2025-04-08T03:14:59Z

@baibaichen could you help to show the log of the failed ClickHouse CI?

And the failed CI run-tpc-test-ubuntu-2204-celeborn seems unrelated to this PR :

25/04/07 09:45:30 ERROR CelebornShuffleReader: Exception caught when readPartition 72!
org.apache.celeborn.common.exception.CelebornIOException: createPartitionReader failed! PartitionLocation[
  id-epoch:72-0
  host-rpcPort-pushPort-fetchPort-replicatePort:172.18.0.2-41173-42161-33177-42025
  mode:PRIMARY
  peer:(empty)
  storage hint:StorageInfo{type=HDD, mountPoint='', finalResult=true, filePath=}
  mapIdBitMap:null]
	at org.apache.celeborn.client.read.CelebornInputStream$CelebornInputStreamImpl.createReaderWithRetry(CelebornInputStream.java:370)
	at org.apache.celeborn.client.read.CelebornInputStream$CelebornInputStreamImpl.moveToNextReader(CelebornInputStream.java:273)
	at org.apache.celeborn.client.read.CelebornInputStream$CelebornInputStreamImpl.<init>(CelebornInputStream.java:222)
	at org.apache.celeborn.client.read.CelebornInputStream.create(CelebornInputStream.java:72)
	at org.apache.celeborn.client.ShuffleClientImpl.readPartition(ShuffleClientImpl.java:1675)
	at org.apache.spark.shuffle.celeborn.CelebornShuffleReader$$anon$3.run(CelebornShuffleReader.scala:125)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.io.IOException: Exception in sendRpcSync to: /172.18.0.2:33177
	at org.apache.celeborn.common.network.client.TransportClient.sendRpcSync(TransportClient.java:324)
	at org.apache.celeborn.client.read.WorkerPartitionReader.<init>(WorkerPartitionReader.java:129)
	at org.apache.celeborn.client.read.CelebornInputStream$CelebornInputStreamImpl.createReader(CelebornInputStream.java:444)
	at org.apache.celeborn.client.read.CelebornInputStream$CelebornInputStreamImpl.createReaderWithRetry(CelebornInputStream.java:341)
	... 10 more
Caused by: java.util.concurrent.ExecutionException: java.io.IOException: org.apache.celeborn.common.exception.PartitionUnRetryAbleException: Could not find file 72-0-0 for local-1744017116595-72.
	at org.apache.celeborn.common.util.ExceptionUtils.wrapIOExceptionToUnRetryable(ExceptionUtils.java:41)
	at org.apache.celeborn.service.deploy.worker.FetchHandler.handleRpcException(FetchHandler.scala:350)
	at org.apache.celeborn.service.deploy.worker.FetchHandler.handleRpcIOException(FetchHandler.scala:342)
	at org.apache.celeborn.service.deploy.worker.FetchHandler.handleOpenStreamInternal(FetchHandler.scala:293)
	at org.apache.celeborn.service.deploy.worker.FetchHandler.handleRpcRequest(FetchHandler.scala:138)
	at org.apache.celeborn.service.deploy.worker.FetchHandler.receive(FetchHandler.scala:97)
	at org.apache.celeborn.common.network.server.TransportRequestHandler.processRpcRequest(TransportRequestHandler.java:96)
	at org.apache.celeborn.common.network.server.TransportRequestHandler.handle(TransportRequestHandler.java:84)
	at org.apache.celeborn.common.network.server.TransportChannelHandler.channelRead(TransportChannelHandler.java:156)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.handler.timeout.IdleStateHandler.channelRead(IdleStateHandler.java:286)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.io.FileNotFoundException: Could not find file 72-0-0 for local-[17440](https://github.com/apache/incubator-gluten/actions/runs/14304948985/job/40086868424?pr=9181#step:7:17441)17116595-72.
	at org.apache.celeborn.service.deploy.worker.FetchHandler.getRawFileInfo(FetchHandler.scala:88)
	at org.apache.celeborn.service.deploy.worker.FetchHandler.handleOpenStreamInternal(FetchHandler.scala:214)
	... 29 more

	at org.apache.celeborn.common.network.client.TransportResponseHandler.handle(TransportResponseHandler.java:390)
	at org.apache.celeborn.common.network.server.TransportChannelHandler.channelRead(TransportChannelHandler.java:158)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at org.apache.celeborn.shaded.io.netty.handler.timeout.IdleStateHandler.channelRead(IdleStateHandler.java:286)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at org.apache.celeborn.common.network.util.TransportFrameDecoder.channelRead(TransportFrameDecoder.java:74)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at org.apache.celeborn.shaded.io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
	at org.apache.celeborn.shaded.io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at org.apache.celeborn.shaded.io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
	at org.apache.celeborn.shaded.io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
	at org.apache.celeborn.shaded.io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788)
	at org.apache.celeborn.shaded.io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724)
	at org.apache.celeborn.shaded.io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650)
	at org.apache.celeborn.shaded.io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
	at org.apache.celeborn.shaded.io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
	at org.apache.celeborn.shaded.io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at org.apache.celeborn.shaded.io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	... 1 more

rui-mo

If the workflow passes.

github-actions · 2025-04-09T02:10:18Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-04-09T06:25:59Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-04-09T08:58:57Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-04-10T02:13:46Z

Run Gluten Clickhouse CI on x86

github-actions · 2025-04-10T06:52:00Z

Run Gluten Clickhouse CI on x86

NEUpanning · 2025-04-10T08:38:23Z

The workflow has finally passed. Please help merge this pull request, @rui-mo.

rui-mo · 2025-04-10T09:12:51Z

Thanks!

github-actions bot added CORE works for Gluten Core VELOX labels Mar 31, 2025

NEUpanning force-pushed the stat_legacy branch from f3ae1c1 to 4169493 Compare April 1, 2025 03:53

jinchengchenghh reviewed Apr 1, 2025

View reviewed changes

rui-mo reviewed Apr 2, 2025

View reviewed changes

NEUpanning force-pushed the stat_legacy branch from dac9a8b to faf5c20 Compare April 7, 2025 03:54

rui-mo reviewed Apr 7, 2025

View reviewed changes

.../spark33/src/test/scala/org/apache/spark/sql/execution/GlutenSQLAggregateFunctionSuite.scala Outdated Show resolved Hide resolved

NEUpanning requested a review from rui-mo April 8, 2025 03:05

rui-mo approved these changes Apr 8, 2025

View reviewed changes

NEUpanning force-pushed the stat_legacy branch from 75a9939 to 1264e82 Compare April 9, 2025 02:09

NEUpanning force-pushed the stat_legacy branch from 038d06b to 2f86824 Compare April 9, 2025 08:58

NEUpanning force-pushed the stat_legacy branch from 2f86824 to a451074 Compare April 10, 2025 02:13

initial

a3985c3

NEUpanning force-pushed the stat_legacy branch from a451074 to a3985c3 Compare April 10, 2025 06:51

rui-mo merged commit 7d11bb6 into apache:main Apr 10, 2025
49 checks passed

NEUpanning deleted the stat_legacy branch April 10, 2025 11:12

Conversation

NEUpanning commented Mar 31, 2025

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

github-actions bot commented Mar 31, 2025

Uh oh!

github-actions bot commented Mar 31, 2025

Uh oh!

github-actions bot commented Apr 1, 2025

Uh oh!

NEUpanning commented Apr 1, 2025

Uh oh!

jinchengchenghh Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

NEUpanning Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

jinchengchenghh Apr 2, 2025

Choose a reason for hiding this comment

Uh oh!

NEUpanning Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jinchengchenghh Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rui-mo left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 3, 2025

Uh oh!

github-actions bot commented Apr 7, 2025

Uh oh!

Uh oh!

github-actions bot commented Apr 7, 2025

Uh oh!

NEUpanning commented Apr 8, 2025

Uh oh!

rui-mo left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 9, 2025

Uh oh!

github-actions bot commented Apr 9, 2025

Uh oh!

github-actions bot commented Apr 9, 2025

Uh oh!

github-actions bot commented Apr 10, 2025

Uh oh!

github-actions bot commented Apr 10, 2025

Uh oh!

NEUpanning commented Apr 10, 2025

Uh oh!

rui-mo commented Apr 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NEUpanning Apr 2, 2025 •

edited

Loading

jinchengchenghh Apr 1, 2025 •

edited

Loading