You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "acvictor (via GitHub)" <gi...@apache.org> on 2025/04/30 12:01:42 UTC
[PR] Update rules [incubator-gluten]
acvictor opened a new pull request, #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473
## What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
(Fixes: \#ISSUE-ID)
## How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#discussion_r2618181454
##########
backends-velox/src/main/scala/org/apache/gluten/execution/HashAggregateExecTransformer.scala:
##########
@@ -716,6 +716,46 @@ case class RegularHashAggregateExecTransformer(
ignoreNullKeys
) {
+ override def isOffloadedSortExec: Boolean = false
+
+ override protected def allowFlush: Boolean = false
+
+ override def simpleString(maxFields: Int): String =
+ s"${super.simpleString(maxFields)}"
+
+ override def verboseString(maxFields: Int): String =
+ s"${super.verboseString(maxFields)}"
+
+ override protected def withNewChildInternal(newChild: SparkPlan): HashAggregateExecTransformer = {
+ copy(child = newChild)
+ }
+}
+
+// Hash aggregation that is offloaded from sort aggregation.
+// Is identical to RegularHashAggregateExecTransformer but with a
+// different value of isOffloadedSortExec.
+case class OffloadedSortHashAggregateExecTransformer(
+ requiredChildDistributionExpressions: Option[Seq[Expression]],
+ groupingExpressions: Seq[NamedExpression],
+ aggregateExpressions: Seq[AggregateExpression],
+ aggregateAttributes: Seq[Attribute],
+ override val initialInputBufferOffset: Int,
+ resultExpressions: Seq[NamedExpression],
+ child: SparkPlan,
+ ignoreNullKeys: Boolean = false)
+ extends HashAggregateExecTransformer(
+ requiredChildDistributionExpressions,
+ groupingExpressions,
+ aggregateExpressions,
+ aggregateAttributes,
+ initialInputBufferOffset,
+ resultExpressions,
+ child,
+ ignoreNullKeys
+ ) {
+
+ override def isOffloadedSortExec: Boolean = true
+
Review Comment:
Apologize for the relay.
If we have to add a boolean flag, how about directly adding `SortAggregateExecTransformer`? Then we can decide whether to remove the sort based on the operator class. It was actually suggested in comment https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2863912230 which I haven't seen is addressed. Would you like to help check this? Thanks.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "rui-mo (via GitHub)" <gi...@apache.org>.
rui-mo commented on code in PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#discussion_r2144875358
##########
backends-velox/src/main/scala/org/apache/gluten/execution/HashAggregateExecTransformer.scala:
##########
@@ -716,6 +716,46 @@ case class RegularHashAggregateExecTransformer(
ignoreNullKeys
) {
+ override def isOffloadedSortExec: Boolean = false
+
+ override protected def allowFlush: Boolean = false
+
+ override def simpleString(maxFields: Int): String =
+ s"${super.simpleString(maxFields)}"
+
+ override def verboseString(maxFields: Int): String =
+ s"${super.verboseString(maxFields)}"
+
+ override protected def withNewChildInternal(newChild: SparkPlan): HashAggregateExecTransformer = {
+ copy(child = newChild)
+ }
+}
+
+// Hash aggregation that is offloaded from sort aggregation.
+// Is identical to RegularHashAggregateExecTransformer but with a
+// different value of isOffloadedSortExec.
+case class OffloadedSortHashAggregateExecTransformer(
Review Comment:
How about the name `HashFromSortAggregateExecTransformer`, which emphasizes it is a hash aggregate but converted from sort aggregate?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2875069317
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor closed pull request #9473: [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate
URL: https://github.com/apache/incubator-gluten/pull/9473
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3935230828
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] Update rules [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2841753150
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICEGJ-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
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](https://github.com/apache/incubator-gluten/pulls/)
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2877317753
> Compared to having this flag, can we just add a new operator for offloaded sort aggregation?
@zhztheplayer can you please review again? Thanks!
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2850390758
@zhztheplayer @NEUpanning can you please help review this PR?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3127986183
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3989872348
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on code in PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#discussion_r2151288508
##########
gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala:
##########
@@ -78,7 +78,8 @@ trait SparkPlanExecApi {
aggregateAttributes: Seq[Attribute],
initialInputBufferOffset: Int,
resultExpressions: Seq[NamedExpression],
- child: SparkPlan): HashAggregateExecBaseTransformer
+ child: SparkPlan,
+ offloadedSortExec: Boolean = false): HashAggregateExecBaseTransformer
Review Comment:
minor: Could just remove the default value if we don't need to maintain backward compatibility for the API.
##########
gluten-substrait/src/main/scala/org/apache/gluten/execution/HashAggregateExecBaseTransformer.scala:
##########
@@ -188,7 +190,9 @@ object HashAggregateExecBaseTransformer {
case a: SortAggregateExec => a.initialInputBufferOffset
}
- def from(agg: BaseAggregateExec): HashAggregateExecBaseTransformer = {
+ def from(
+ agg: BaseAggregateExec,
+ offloadedSortExec: Boolean = false): HashAggregateExecBaseTransformer = {
Review Comment:
ditto
##########
backends-velox/src/main/scala/org/apache/gluten/execution/HashAggregateExecTransformer.scala:
##########
@@ -716,6 +716,46 @@ case class RegularHashAggregateExecTransformer(
ignoreNullKeys
) {
+ override def isOffloadedSortExec: Boolean = false
+
+ override protected def allowFlush: Boolean = false
+
+ override def simpleString(maxFields: Int): String =
+ s"${super.simpleString(maxFields)}"
+
+ override def verboseString(maxFields: Int): String =
+ s"${super.verboseString(maxFields)}"
+
+ override protected def withNewChildInternal(newChild: SparkPlan): HashAggregateExecTransformer = {
+ copy(child = newChild)
+ }
+}
+
+// Hash aggregation that is offloaded from sort aggregation.
+// Is identical to RegularHashAggregateExecTransformer but with a
+// different value of isOffloadedSortExec.
+case class OffloadedSortHashAggregateExecTransformer(
+ requiredChildDistributionExpressions: Option[Seq[Expression]],
+ groupingExpressions: Seq[NamedExpression],
+ aggregateExpressions: Seq[AggregateExpression],
+ aggregateAttributes: Seq[Attribute],
+ override val initialInputBufferOffset: Int,
+ resultExpressions: Seq[NamedExpression],
+ child: SparkPlan,
+ ignoreNullKeys: Boolean = false)
+ extends HashAggregateExecTransformer(
+ requiredChildDistributionExpressions,
+ groupingExpressions,
+ aggregateExpressions,
+ aggregateAttributes,
+ initialInputBufferOffset,
+ resultExpressions,
+ child,
+ ignoreNullKeys
+ ) {
+
+ override def isOffloadedSortExec: Boolean = true
+
Review Comment:
Let's override the `requiredChildOrdering` and `outputOrdering` so that this API doesn't have to be added.
The implementation may be the same with Spark's `SortAggrtegateExec`:
https://github.com/apache/spark/blob/abecd4affbd9102d73434caf1f1ca00bda9ef6fe/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/SortAggregateExec.scala#L50-L56
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "rui-mo (via GitHub)" <gi...@apache.org>.
rui-mo commented on code in PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#discussion_r2873269443
##########
backends-velox/src/main/scala/org/apache/gluten/extension/FlushableHashAggregateRule.scala:
##########
@@ -83,7 +83,7 @@ case class FlushableHashAggregateRule(session: SparkSession) extends Rule[SparkP
}
private def replaceEligibleAggregates(plan: SparkPlan)(
- func: RegularHashAggregateExecTransformer => SparkPlan): SparkPlan = {
+ func: HashAggregateExecTransformer => SparkPlan): SparkPlan = {
Review Comment:
Could you please add some comments for this function for readability? Thanks.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3989905194
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] Update rules [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2844696826
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3127325289
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] Update rules [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2841753708
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2875912170
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2870842647
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2870792305
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2870919892
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3938089111
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3067793843
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2875069085
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2870864702
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2863912230
Compared to having this flag, can we just add a new operator for offloaded sort aggregation?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3652609659
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2866074842
> Compared to having this flag, can we just add a new operator for offloaded sort aggregation?
Thanks for the suggestion. I will try it out.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3067994341
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3157337917
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] Update rules [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2841859951
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "FelixYBW (via GitHub)" <gi...@apache.org>.
FelixYBW commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3466421031
@zhztheplayer Any more comments?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3467020688
Apologize for the delay here! I'll revisit this shortly. Please feel free to jump in and review if anyone likes to.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] Update rules [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2845182912
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] Update rules [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2844701025
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3067903710
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3068879158
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3989871220
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3991284633
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on code in PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#discussion_r2332460111
##########
backends-velox/src/main/scala/org/apache/gluten/execution/HashAggregateExecTransformer.scala:
##########
@@ -716,6 +716,46 @@ case class RegularHashAggregateExecTransformer(
ignoreNullKeys
) {
+ override def isOffloadedSortExec: Boolean = false
+
+ override protected def allowFlush: Boolean = false
+
+ override def simpleString(maxFields: Int): String =
+ s"${super.simpleString(maxFields)}"
+
+ override def verboseString(maxFields: Int): String =
+ s"${super.verboseString(maxFields)}"
+
+ override protected def withNewChildInternal(newChild: SparkPlan): HashAggregateExecTransformer = {
+ copy(child = newChild)
+ }
+}
+
+// Hash aggregation that is offloaded from sort aggregation.
+// Is identical to RegularHashAggregateExecTransformer but with a
+// different value of isOffloadedSortExec.
+case class OffloadedSortHashAggregateExecTransformer(
+ requiredChildDistributionExpressions: Option[Seq[Expression]],
+ groupingExpressions: Seq[NamedExpression],
+ aggregateExpressions: Seq[AggregateExpression],
+ aggregateAttributes: Seq[Attribute],
+ override val initialInputBufferOffset: Int,
+ resultExpressions: Seq[NamedExpression],
+ child: SparkPlan,
+ ignoreNullKeys: Boolean = false)
+ extends HashAggregateExecTransformer(
+ requiredChildDistributionExpressions,
+ groupingExpressions,
+ aggregateExpressions,
+ aggregateAttributes,
+ initialInputBufferOffset,
+ resultExpressions,
+ child,
+ ignoreNullKeys
+ ) {
+
+ override def isOffloadedSortExec: Boolean = true
+
Review Comment:
> Let's override the `requiredChildOrdering` and `outputOrdering` so that this API doesn't have to be added.
>
> The implementation may be the same with Spark's `SortAggrtegateExec`:
>
> https://github.com/apache/spark/blob/abecd4affbd9102d73434caf1f1ca00bda9ef6fe/sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/SortAggregateExec.scala#L50-L56
@zhztheplayer I tried this approach but it does eliminate the sort in some cases, for example `drop redundant partial sort which has pre-project when offload sortAgg`. Do you think it is okay to retain the bool API?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on code in PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#discussion_r2236597560
##########
backends-velox/src/main/scala/org/apache/gluten/execution/HashAggregateExecTransformer.scala:
##########
@@ -716,6 +716,46 @@ case class RegularHashAggregateExecTransformer(
ignoreNullKeys
) {
+ override def isOffloadedSortExec: Boolean = false
+
+ override protected def allowFlush: Boolean = false
+
+ override def simpleString(maxFields: Int): String =
+ s"${super.simpleString(maxFields)}"
+
+ override def verboseString(maxFields: Int): String =
+ s"${super.verboseString(maxFields)}"
+
+ override protected def withNewChildInternal(newChild: SparkPlan): HashAggregateExecTransformer = {
+ copy(child = newChild)
+ }
+}
+
+// Hash aggregation that is offloaded from sort aggregation.
+// Is identical to RegularHashAggregateExecTransformer but with a
+// different value of isOffloadedSortExec.
+case class OffloadedSortHashAggregateExecTransformer(
+ requiredChildDistributionExpressions: Option[Seq[Expression]],
+ groupingExpressions: Seq[NamedExpression],
+ aggregateExpressions: Seq[AggregateExpression],
+ aggregateAttributes: Seq[Attribute],
+ override val initialInputBufferOffset: Int,
+ resultExpressions: Seq[NamedExpression],
+ child: SparkPlan,
+ ignoreNullKeys: Boolean = false)
+ extends HashAggregateExecTransformer(
+ requiredChildDistributionExpressions,
+ groupingExpressions,
+ aggregateExpressions,
+ aggregateAttributes,
+ initialInputBufferOffset,
+ resultExpressions,
+ child,
+ ignoreNullKeys
+ ) {
+
+ override def isOffloadedSortExec: Boolean = true
+
Review Comment:
Do you mean `requiredChildOrdering` and `orderingExpressions`?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on code in PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#discussion_r2338501334
##########
backends-velox/src/main/scala/org/apache/gluten/execution/HashAggregateExecTransformer.scala:
##########
@@ -716,6 +716,46 @@ case class RegularHashAggregateExecTransformer(
ignoreNullKeys
) {
+ override def isOffloadedSortExec: Boolean = false
+
+ override protected def allowFlush: Boolean = false
+
+ override def simpleString(maxFields: Int): String =
+ s"${super.simpleString(maxFields)}"
+
+ override def verboseString(maxFields: Int): String =
+ s"${super.verboseString(maxFields)}"
+
+ override protected def withNewChildInternal(newChild: SparkPlan): HashAggregateExecTransformer = {
+ copy(child = newChild)
+ }
+}
+
+// Hash aggregation that is offloaded from sort aggregation.
+// Is identical to RegularHashAggregateExecTransformer but with a
+// different value of isOffloadedSortExec.
+case class OffloadedSortHashAggregateExecTransformer(
+ requiredChildDistributionExpressions: Option[Seq[Expression]],
+ groupingExpressions: Seq[NamedExpression],
+ aggregateExpressions: Seq[AggregateExpression],
+ aggregateAttributes: Seq[Attribute],
+ override val initialInputBufferOffset: Int,
+ resultExpressions: Seq[NamedExpression],
+ child: SparkPlan,
+ ignoreNullKeys: Boolean = false)
+ extends HashAggregateExecTransformer(
+ requiredChildDistributionExpressions,
+ groupingExpressions,
+ aggregateExpressions,
+ aggregateAttributes,
+ initialInputBufferOffset,
+ resultExpressions,
+ child,
+ ignoreNullKeys
+ ) {
+
+ override def isOffloadedSortExec: Boolean = true
+
Review Comment:
@zhztheplayer I also realized that we can have flushable aggregate from sort aggregate too. Is it better to just add a boolean to HashAggregateExecTransformer? Else I will have to add another FlushableHashFromSortAggregateExec.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2870768102
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2846838383
https://github.com/apache/incubator-gluten/issues/8227
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] Update rules [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2846313225
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3935217672
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3068318345
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3269468996
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3127524876
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on code in PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#discussion_r2238572754
##########
gluten-substrait/src/main/scala/org/apache/gluten/execution/HashAggregateExecBaseTransformer.scala:
##########
@@ -188,7 +190,9 @@ object HashAggregateExecBaseTransformer {
case a: SortAggregateExec => a.initialInputBufferOffset
}
- def from(agg: BaseAggregateExec): HashAggregateExecBaseTransformer = {
+ def from(
+ agg: BaseAggregateExec,
+ offloadedSortExec: Boolean = false): HashAggregateExecBaseTransformer = {
Review Comment:
Done.
##########
gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/SparkPlanExecApi.scala:
##########
@@ -78,7 +78,8 @@ trait SparkPlanExecApi {
aggregateAttributes: Seq[Attribute],
initialInputBufferOffset: Int,
resultExpressions: Seq[NamedExpression],
- child: SparkPlan): HashAggregateExecBaseTransformer
+ child: SparkPlan,
+ offloadedSortExec: Boolean = false): HashAggregateExecBaseTransformer
Review Comment:
Done.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on code in PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#discussion_r2204103802
##########
backends-velox/src/main/scala/org/apache/gluten/execution/HashAggregateExecTransformer.scala:
##########
@@ -716,6 +716,46 @@ case class RegularHashAggregateExecTransformer(
ignoreNullKeys
) {
+ override def isOffloadedSortExec: Boolean = false
+
+ override protected def allowFlush: Boolean = false
+
+ override def simpleString(maxFields: Int): String =
+ s"${super.simpleString(maxFields)}"
+
+ override def verboseString(maxFields: Int): String =
+ s"${super.verboseString(maxFields)}"
+
+ override protected def withNewChildInternal(newChild: SparkPlan): HashAggregateExecTransformer = {
+ copy(child = newChild)
+ }
+}
+
+// Hash aggregation that is offloaded from sort aggregation.
+// Is identical to RegularHashAggregateExecTransformer but with a
+// different value of isOffloadedSortExec.
+case class OffloadedSortHashAggregateExecTransformer(
Review Comment:
Done.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3449239073
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-3821386885
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on PR #9473:
URL: https://github.com/apache/gluten/pull/9473#issuecomment-4066994818
@zhztheplayer @rui-mo can you please review this again? The main change was in response to the comment "how about directly adding SortAggregateExecTransformer? Then we can decide whether to remove the sort based on the operator class."
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [gluten]
Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer merged PR #9473:
URL: https://github.com/apache/gluten/pull/9473
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2981194657
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2870768134
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2870775857
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2870777421
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2967508343
@rui-mo can you please review this PR?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] Update rules [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2844949197
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] Update rules [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2846258549
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] Update rules [incubator-gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2842925849
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2863362377
@ulysses-you can you please take a look at this PR?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [incubator-gluten]
Posted by "acvictor (via GitHub)" <gi...@apache.org>.
acvictor commented on code in PR #9473:
URL: https://github.com/apache/incubator-gluten/pull/9473#discussion_r2835855382
##########
backends-velox/src/main/scala/org/apache/gluten/execution/HashAggregateExecTransformer.scala:
##########
@@ -716,6 +716,46 @@ case class RegularHashAggregateExecTransformer(
ignoreNullKeys
) {
+ override def isOffloadedSortExec: Boolean = false
+
+ override protected def allowFlush: Boolean = false
+
+ override def simpleString(maxFields: Int): String =
+ s"${super.simpleString(maxFields)}"
+
+ override def verboseString(maxFields: Int): String =
+ s"${super.verboseString(maxFields)}"
+
+ override protected def withNewChildInternal(newChild: SparkPlan): HashAggregateExecTransformer = {
+ copy(child = newChild)
+ }
+}
+
+// Hash aggregation that is offloaded from sort aggregation.
+// Is identical to RegularHashAggregateExecTransformer but with a
+// different value of isOffloadedSortExec.
+case class OffloadedSortHashAggregateExecTransformer(
+ requiredChildDistributionExpressions: Option[Seq[Expression]],
+ groupingExpressions: Seq[NamedExpression],
+ aggregateExpressions: Seq[AggregateExpression],
+ aggregateAttributes: Seq[Attribute],
+ override val initialInputBufferOffset: Int,
+ resultExpressions: Seq[NamedExpression],
+ child: SparkPlan,
+ ignoreNullKeys: Boolean = false)
+ extends HashAggregateExecTransformer(
+ requiredChildDistributionExpressions,
+ groupingExpressions,
+ aggregateExpressions,
+ aggregateAttributes,
+ initialInputBufferOffset,
+ resultExpressions,
+ child,
+ ignoreNullKeys
+ ) {
+
+ override def isOffloadedSortExec: Boolean = true
+
Review Comment:
> Apologize for the relay.
>
> If we have to add a boolean flag, how about directly adding `SortAggregateExecTransformer`? Then we can decide whether to remove the sort based on the operator class. It was actually suggested in comment [#9473 (comment)](https://github.com/apache/incubator-gluten/pull/9473#issuecomment-2863912230) which I haven't seen is addressed. Would you like to help check this? Thanks.
@zhztheplayer sorry for dropping the ball on this. I have updated the PR based on your comments. Can you please take a look?
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-8227][VL] fix: Update sort elimination rules for Hash Aggregate [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #9473:
URL: https://github.com/apache/gluten/pull/9473#issuecomment-4102209273
Run Gluten Clickhouse CI on x86
--
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: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org