Clean up code

apache · Mar 19, 2024 · c776eb6 · c776eb6
1 parent 1648b3d
commit c776eb6
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 19 deletions.
diff --git a/gluten-core/src/main/scala/io/glutenproject/backendsapi/SparkPlanExecApi.scala b/gluten-core/src/main/scala/io/glutenproject/backendsapi/SparkPlanExecApi.scala
@@ -520,25 +520,10 @@ trait SparkPlanExecApi {
                   attributeSeq = originalInputAttributes)
                 .doTransform(args))
             // Spark only accepts foldable offset. Converts it to LongType literal.
-            var offset = offsetWf.offset.eval(EmptyRow).asInstanceOf[Int]
-//            if (wf.isInstanceOf[Lead]) {
-//              if (offset < 0) {
-//                // Velox always expects non-negative offset.
-//                throw new GlutenNotSupportException(
-//                  s"${wf.nodeName} does not support negative offset: $offset")
-//              }
-//            } else {
-//              // For Lag
-//              // Spark would use `-inputOffset` as offset, so here we forbid positive offset.
-//              // Which means the inputOffset is negative.
-//              if (offset > 0) {
-//                // Velox always expects non-negative offset.
-//                throw new GlutenNotSupportException(
-//                  s"${wf.nodeName} does not support negative offset: $offset")
-//              }
-//              // Revert the Spark change and use the original input offset
-//              offset = -offset
-//            }
+            val offset = offsetWf.offset.eval(EmptyRow).asInstanceOf[Int]
+            // Velox only allows negative offset. WindowFunctionsBuilder#create converts
+            // lag/lead with negative offset to the function with positive offset. So just
+            // makes offsetNode store positive value.
             val offsetNode = ExpressionBuilder.makeLiteral(Math.abs(offset.toLong), LongType, false)
             childrenNodeList.add(offsetNode)
             // NullType means Null is the default value. Don't pass it to native.

diff --git a/gluten-core/src/main/scala/io/glutenproject/expression/WindowFunctionsBuilder.scala b/gluten-core/src/main/scala/io/glutenproject/expression/WindowFunctionsBuilder.scala
@@ -29,8 +29,11 @@ object WindowFunctionsBuilder {
   def create(args: java.lang.Object, windowFunc: WindowFunction): Long = {
     val functionMap = args.asInstanceOf[java.util.HashMap[String, java.lang.Long]]
     val substraitFunc = windowFunc match {
+      // Handle lag with negative inputOffset, e.g., converts lag(c1, -1) to lead(c1, 1).
+      // Spark uses `-inputOffset` as lag's offset.
       case lag: Lag if lag.offset.eval(EmptyRow).asInstanceOf[Int] > 0 =>
         Some(LEAD)
+      // Handle lead with negative offset, e.g., converts lead(c1, -1) to lag(c1, 1).
       case lead: Lead if lead.offset.eval(EmptyRow).asInstanceOf[Int] < 0 =>
         Some(LAG)
       case _ =>