Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-20 Thread via GitHub 


beliefer commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3279017458

   ping @zhztheplayer @PHILO-HE @zml1206 @zhouyuan 


-- 
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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-19 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3279884002

   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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-18 Thread via GitHub 


PHILO-HE commented on code in PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#discussion_r2343799522


##
shims/spark35/src/main/scala/org/apache/gluten/sql/shims/spark35/Spark35Shims.scala:
##
@@ -304,24 +304,32 @@ class Spark35Shims extends SparkShims {
 
   override def getWindowGroupLimitExecShim(plan: SparkPlan): 
WindowGroupLimitExecShim = {
 val windowGroupLimitPlan = plan.asInstanceOf[WindowGroupLimitExec]
+val mode = windowGroupLimitPlan.mode match {
+  case Partial => GlutenPartial
+  case Final => GlutenFinal
+}

Review Comment:
   Maybe, we can define Scala implicit conversions in shims/common, then import 
them here for implicit conversion between Partial and GlutenPartial at compile 
time. This can simplify the code a bit. Please consider whether it is worth 
doing.
   



-- 
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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-17 Thread via GitHub 


beliefer commented on code in PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#discussion_r2343815567


##
shims/spark35/src/main/scala/org/apache/gluten/sql/shims/spark35/Spark35Shims.scala:
##
@@ -304,24 +304,32 @@ class Spark35Shims extends SparkShims {
 
   override def getWindowGroupLimitExecShim(plan: SparkPlan): 
WindowGroupLimitExecShim = {
 val windowGroupLimitPlan = plan.asInstanceOf[WindowGroupLimitExec]
+val mode = windowGroupLimitPlan.mode match {
+  case Partial => GlutenPartial
+  case Final => GlutenFinal
+}

Review Comment:
   Could I make this change in another 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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-17 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3274352221

   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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-17 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3280227597

   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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-12 Thread via GitHub 


beliefer commented on code in PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#discussion_r2343908457


##
shims/spark35/src/main/scala/org/apache/gluten/sql/shims/spark35/Spark35Shims.scala:
##
@@ -304,24 +304,32 @@ class Spark35Shims extends SparkShims {
 
   override def getWindowGroupLimitExecShim(plan: SparkPlan): 
WindowGroupLimitExecShim = {
 val windowGroupLimitPlan = plan.asInstanceOf[WindowGroupLimitExec]
+val mode = windowGroupLimitPlan.mode match {
+  case Partial => GlutenPartial
+  case Final => GlutenFinal
+}

Review Comment:
   It is not possible to add Scala implicit conversions in shims/common due to 
not all Spark versions exists `WindowGroupLimitMode`.



-- 
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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-12 Thread via GitHub 


beliefer commented on code in PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#discussion_r2343815567


##
shims/spark35/src/main/scala/org/apache/gluten/sql/shims/spark35/Spark35Shims.scala:
##
@@ -304,24 +304,32 @@ class Spark35Shims extends SparkShims {
 
   override def getWindowGroupLimitExecShim(plan: SparkPlan): 
WindowGroupLimitExecShim = {
 val windowGroupLimitPlan = plan.asInstanceOf[WindowGroupLimitExec]
+val mode = windowGroupLimitPlan.mode match {
+  case Partial => GlutenPartial
+  case Final => GlutenFinal
+}

Review Comment:
   Could I make this change in another 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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-12 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3274351626

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


-- 
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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-12 Thread via GitHub 


beliefer commented on code in PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#discussion_r2343230279


##
shims/common/src/main/scala/org/apache/spark/sql/execution/window/WindowGroupLimitExecShim.scala:
##
@@ -21,18 +21,18 @@ import org.apache.spark.sql.catalyst.InternalRow
 import org.apache.spark.sql.catalyst.expressions.{Attribute, Expression, 
SortOrder}
 import org.apache.spark.sql.execution.{SparkPlan, UnaryExecNode}
 
-sealed trait WindowGroupLimitMode
+sealed trait GlutenWindowGroupLimitMode
 
-case object Partial extends WindowGroupLimitMode
+case object GlutenPartial extends GlutenWindowGroupLimitMode
 
-case object Final extends WindowGroupLimitMode
+case object GlutenFinal extends GlutenWindowGroupLimitMode
 
 case class WindowGroupLimitExecShim(
 partitionSpec: Seq[Expression],
 orderSpec: Seq[SortOrder],
 rankLikeFunction: Expression,
 limit: Int,
-mode: WindowGroupLimitMode,
+mode: GlutenWindowGroupLimitMode,
 child: SparkPlan)
   extends UnaryExecNode {

Review Comment:
   Because `PullOutPreProject` calls `copyTagsFrom`, I restored  
`WindowGroupLimitExecShim`.



-- 
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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-12 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3284051974

   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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-11 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3274754429

   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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-11 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3279938677

   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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-11 Thread via GitHub 


zml1206 commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3283980843

   ```
   2025-09-12T05:09:26.6036400Z - Gluten - Filter on row number *** FAILED ***
   2025-09-12T05:09:26.6037604Z   List(SortExecTransformer [c_nationkey#560179 
ASC NULLS FIRST, _pre_712#560200 ASC NULLS FIRST, c_custkey#560178 ASC NULLS 
FIRST, _pre_712#560200 ASC NULLS FIRST], false, 0
   2025-09-12T05:09:26.6038999Z   +- ProjectExecTransformer [c_custkey#560178, 
c_acctbal#560180, c_nationkey#560179, a AS _pre_712#560200]
   2025-09-12T05:09:26.6040008Z  +- 
InputIteratorTransformer[c_custkey#560178, c_nationkey#560179, c_acctbal#560180]
   2025-09-12T05:09:26.6040681Z +- RowToVeloxColumnar
   2025-09-12T05:09:26.6041532Z+- *(1) Scan 
ExistingRDD[c_custkey#560178,c_nationkey#560179,c_acctbal#560180]
   2025-09-12T05:09:26.6043078Z   , SortExecTransformer [c_nationkey#560179 ASC 
NULLS FIRST, _pre_716#560205 ASC NULLS FIRST, c_custkey#560178 ASC NULLS FIRST, 
_pre_716#560205 ASC NULLS FIRST], false, 0
   2025-09-12T05:09:26.6044504Z   +- ProjectExecTransformer [c_custkey#560178, 
c_acctbal#560180, c_nationkey#560179, a AS _pre_716#560205]
   2025-09-12T05:09:26.6045875Z  +- WindowGroupLimitExecTransformer 
[c_nationkey#560179, _pre_715#560204], [c_custkey#560178 ASC NULLS FIRST, 
_pre_715#560204 ASC NULLS FIRST], row_number(), 2, GlutenFinal
   2025-09-12T05:09:26.6047260Z +- ProjectExecTransformer 
[c_custkey#560178, c_acctbal#560180, c_nationkey#560179, a AS _pre_715#560204]
   2025-09-12T05:09:26.6048278Z+- 
InputIteratorTransformer[c_custkey#560178, c_acctbal#560180, c_nationkey#560179]
   2025-09-12T05:09:26.6048946Z   +- ShuffleQueryStage 0
   2025-09-12T05:09:26.6051350Z  +- ColumnarExchange 
hashpartitioning(c_nationkey#560179, a, 1), ENSURE_REQUIREMENTS, 
[c_custkey#560178, c_acctbal#560180, c_nationkey#560179], [plan_id=20910903], 
[shuffle_writer_type=hash], [OUTPUT] List(c_custkey:IntegerType, 
c_acctbal:DecimalType(7,2), c_nationkey:IntegerType), [OUTPUT] 
List(c_custkey:IntegerType, c_acctbal:DecimalType(7,2), c_nationkey:IntegerType)
   2025-09-12T05:09:26.6053540Z +- VeloxResizeBatches 1024, 
2147483647
   2025-09-12T05:09:26.6054544Z+- ^(52516) 
ProjectExecTransformer [hash(c_nationkey#560179, a, 42) AS 
hash_partition_key#560202, c_custkey#560178, c_acctbal#560180, 
c_nationkey#560179]
   2025-09-12T05:09:26.6055753Z   +- ^(52516) 
InputIteratorTransformer[c_custkey#560178, c_acctbal#560180, c_nationkey#560179]
   2025-09-12T05:09:26.6056653Z  +- 
RowToVeloxColumnar
   2025-09-12T05:09:26.6057554Z +- 
WindowGroupLimit [c_nationkey#560179, a], [c_custkey#560178 ASC NULLS FIRST, a 
ASC NULLS FIRST], row_number(), 2, Partial
   2025-09-12T05:09:26.6058375Z+- 
VeloxColumnarToRow
   2025-09-12T05:09:26.6059085Z   +- 
^(52515) ProjectExecTransformer [c_custkey#560178, c_acctbal#560180, 
c_nationkey#560179]
   2025-09-12T05:09:26.6060631Z  +- 
^(52515) SortExecTransformer [c_nationkey#560179 ASC NULLS FIRST, 
_pre_712#560200 ASC NULLS FIRST, c_custkey#560178 ASC NULLS FIRST, 
_pre_712#560200 ASC NULLS FIRST], false, 0
   2025-09-12T05:09:26.6062370Z +- 
^(52515) ProjectExecTransformer [c_custkey#560178, c_acctbal#560180, 
c_nationkey#560179, a AS _pre_712#560200]
   2025-09-12T05:09:26.6063494Z
+- ^(52515) InputIteratorTransformer[c_custkey#560178, c_nationkey#560179, 
c_acctbal#560180]
   2025-09-12T05:09:26.6064274Z 
  +- RowToVeloxColumnar
   2025-09-12T05:09:26.6064941Z 
 +- *(1) Scan 
ExistingRDD[c_custkey#560178,c_nationkey#560179,c_acctbal#560180]
   2025-09-12T05:09:26.6065989Z   ) had size 2 instead of expected size 1 
(GlutenSQLWindowFunctionSuite.scala:137)
   ```


-- 
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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-11 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3280347631

   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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-11 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3279798351

   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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-11 Thread via GitHub 


zhztheplayer commented on code in PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#discussion_r2339963347


##
shims/common/src/main/scala/org/apache/spark/sql/execution/window/WindowGroupLimitExecShim.scala:
##
@@ -21,18 +21,18 @@ import org.apache.spark.sql.catalyst.InternalRow
 import org.apache.spark.sql.catalyst.expressions.{Attribute, Expression, 
SortOrder}
 import org.apache.spark.sql.execution.{SparkPlan, UnaryExecNode}
 
-sealed trait WindowGroupLimitMode
+sealed trait GlutenWindowGroupLimitMode
 
-case object Partial extends WindowGroupLimitMode
+case object GlutenPartial extends GlutenWindowGroupLimitMode
 
-case object Final extends WindowGroupLimitMode
+case object GlutenFinal extends GlutenWindowGroupLimitMode
 
 case class WindowGroupLimitExecShim(
 partitionSpec: Seq[Expression],
 orderSpec: Seq[SortOrder],
 rankLikeFunction: Expression,
 limit: Int,
-mode: WindowGroupLimitMode,
+mode: GlutenWindowGroupLimitMode,
 child: SparkPlan)
   extends UnaryExecNode {

Review Comment:
   Do we really have to make WindowGroupLimitExecShim extend `UnaryExecNode`? 
Can we turn it into a normal scala bean class? E.g.,
   
   ```scala
   case class WindowGroupLimitExecShim(
   partitionSpec: Seq[Expression],
   orderSpec: Seq[SortOrder],
   rankLikeFunction: Expression,
   limit: Int,
   mode: GlutenWindowGroupLimitMode,
   child: SparkPlan)
   ```
   
   Is this sufficient? If yes, it can slightly reduce the code complexity.



-- 
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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-11 Thread via GitHub 


zhztheplayer commented on code in PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#discussion_r2339953178


##
shims/common/src/main/scala/org/apache/spark/sql/execution/window/WindowGroupLimitExecShim.scala:
##
@@ -21,18 +21,18 @@ import org.apache.spark.sql.catalyst.InternalRow
 import org.apache.spark.sql.catalyst.expressions.{Attribute, Expression, 
SortOrder}
 import org.apache.spark.sql.execution.{SparkPlan, UnaryExecNode}
 
-sealed trait WindowGroupLimitMode
+sealed trait GlutenWindowGroupLimitMode
 
-case object Partial extends WindowGroupLimitMode
+case object GlutenPartial extends GlutenWindowGroupLimitMode
 
-case object Final extends WindowGroupLimitMode
+case object GlutenFinal extends GlutenWindowGroupLimitMode

Review Comment:
   Thanks for the explanation.



-- 
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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-11 Thread via GitHub 


beliefer commented on code in PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#discussion_r2339802756


##
shims/common/src/main/scala/org/apache/spark/sql/execution/window/WindowGroupLimitExecShim.scala:
##
@@ -21,18 +21,18 @@ import org.apache.spark.sql.catalyst.InternalRow
 import org.apache.spark.sql.catalyst.expressions.{Attribute, Expression, 
SortOrder}
 import org.apache.spark.sql.execution.{SparkPlan, UnaryExecNode}
 
-sealed trait WindowGroupLimitMode
+sealed trait GlutenWindowGroupLimitMode
 
-case object Partial extends WindowGroupLimitMode
+case object GlutenPartial extends GlutenWindowGroupLimitMode
 
-case object Final extends WindowGroupLimitMode
+case object GlutenFinal extends GlutenWindowGroupLimitMode

Review Comment:
   Because Spark 3.2 and Spark 3.3 don't have `WindowGroupLimitMode`, Spark 3.4 
and later versions added this trait. Gluten added the `WindowGroupLimitMode` 
with the same name as Spark did. I think the author want to solve the 
compilation issue that Spark 3.2 and 3.3 missing `WindowGroupLimitMode`.
   For sharing `WindowGroupLimitExecShim`, rename them.



-- 
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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-11 Thread via GitHub 


beliefer commented on code in PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#discussion_r2339802756


##
shims/common/src/main/scala/org/apache/spark/sql/execution/window/WindowGroupLimitExecShim.scala:
##
@@ -21,18 +21,18 @@ import org.apache.spark.sql.catalyst.InternalRow
 import org.apache.spark.sql.catalyst.expressions.{Attribute, Expression, 
SortOrder}
 import org.apache.spark.sql.execution.{SparkPlan, UnaryExecNode}
 
-sealed trait WindowGroupLimitMode
+sealed trait GlutenWindowGroupLimitMode
 
-case object Partial extends WindowGroupLimitMode
+case object GlutenPartial extends GlutenWindowGroupLimitMode
 
-case object Final extends WindowGroupLimitMode
+case object GlutenFinal extends GlutenWindowGroupLimitMode

Review Comment:
   Because Spark 3.2 and Spark 3.3 don't have `WindowGroupLimitMode`, Spark 3.4 
and later versions added this trait. Gluten added the WindowGroupLimitMode with 
the same name as Spark did. I think the author want to solve the compilation 
issue that Spark 3.2 and 3.3 missing `WindowGroupLimitMode`.
   For sharing `WindowGroupLimitExecShim`, rename them.



-- 
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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-11 Thread via GitHub 


zhztheplayer commented on code in PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#discussion_r2339695189


##
shims/common/src/main/scala/org/apache/spark/sql/execution/window/WindowGroupLimitExecShim.scala:
##
@@ -21,18 +21,18 @@ import org.apache.spark.sql.catalyst.InternalRow
 import org.apache.spark.sql.catalyst.expressions.{Attribute, Expression, 
SortOrder}
 import org.apache.spark.sql.execution.{SparkPlan, UnaryExecNode}
 
-sealed trait WindowGroupLimitMode
+sealed trait GlutenWindowGroupLimitMode
 
-case object Partial extends WindowGroupLimitMode
+case object GlutenPartial extends GlutenWindowGroupLimitMode
 
-case object Final extends WindowGroupLimitMode
+case object GlutenFinal extends GlutenWindowGroupLimitMode

Review Comment:
   Would you share the reason for renaming? 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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-10 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3274852476

   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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-10 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3277146467

   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: [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]

Re: [PR] [GLUTEN-10668] Share WindowGroupLimitExecShim between different spark shims [incubator-gluten]

2025-09-10 Thread via GitHub 


github-actions[bot] commented on PR #10669:
URL: 
https://github.com/apache/incubator-gluten/pull/10669#issuecomment-3274681350

   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: [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]