WangGuangxin commented on code in PR #5626:
URL: https://github.com/apache/incubator-gluten/pull/5626#discussion_r1620692238


##########
gluten-core/src/main/java/org/apache/gluten/substrait/expression/WindowFunctionNode.java:
##########
@@ -80,20 +92,53 @@ private Expression.WindowFunction.Bound.Builder setBound(
         builder.setUnboundedFollowing(followingBuilder.build());
         break;
       default:
-        try {
-          Long offset = Long.valueOf(boundType);
+        if (boundType instanceof PreComputeRangeFrameBound) {
+          // Used only when backend is velox and frame type is RANGE.
+          if (frameType != "RANGE") {
+            throw new GlutenException(
+                "Only Range frame supports PreComputeRangeFrameBound, but got 
" + frameType);
+          }
+          ExpressionNode refNode =
+              ExpressionConverter.replaceWithExpressionTransformer(
+                      ((PreComputeRangeFrameBound) 
boundType).child().toAttribute(),
+                      
JavaConverters.asScalaIteratorConverter(originalInputAttributes.iterator())
+                          .asScala()
+                          .toSeq())
+                  .doTransform(new HashMap<String, Long>());
+          Long offset = Long.valueOf(boundType.eval(null).toString());
           if (offset < 0) {
-            Expression.WindowFunction.Bound.Preceding.Builder 
offsetPrecedingBuilder =
+            Expression.WindowFunction.Bound.Preceding.Builder 
refPrecedingBuilder =
                 Expression.WindowFunction.Bound.Preceding.newBuilder();
-            offsetPrecedingBuilder.setOffset(0 - offset);
-            builder.setPreceding(offsetPrecedingBuilder.build());
+            refPrecedingBuilder.setRef(refNode.toProtobuf());
+            builder.setPreceding(refPrecedingBuilder.build());
           } else {
-            Expression.WindowFunction.Bound.Following.Builder 
offsetFollowingBuilder =
+            Expression.WindowFunction.Bound.Following.Builder 
refFollowingBuilder =
                 Expression.WindowFunction.Bound.Following.newBuilder();
-            offsetFollowingBuilder.setOffset(offset);
-            builder.setFollowing(offsetFollowingBuilder.build());
+            refFollowingBuilder.setRef(refNode.toProtobuf());
+            builder.setFollowing(refFollowingBuilder.build());
+          }
+        } else if (boundType.foldable()) {
+          // Used when
+          // 1. Velox backend and frame type is ROW
+          // 2. Clickhouse backend
+          try {
+            Long offset = Long.valueOf(boundType.eval(null).toString());
+            if (offset < 0) {
+              Expression.WindowFunction.Bound.Preceding.Builder 
offsetPrecedingBuilder =
+                  Expression.WindowFunction.Bound.Preceding.newBuilder();
+              offsetPrecedingBuilder.setOffset(0 - offset);
+              builder.setPreceding(offsetPrecedingBuilder.build());
+            } else {
+              Expression.WindowFunction.Bound.Following.Builder 
offsetFollowingBuilder =
+                  Expression.WindowFunction.Bound.Following.newBuilder();
+              offsetFollowingBuilder.setOffset(offset);
+              builder.setFollowing(offsetFollowingBuilder.build());
+            }
+          } catch (NumberFormatException e) {
+            throw new UnsupportedOperationException(
+                "Unsupported Window Function Frame Type:" + boundType);
           }
-        } catch (NumberFormatException e) {
+        } else {
           throw new UnsupportedOperationException(
               "Unsupported Window Function Frame Type:" + boundType);

Review Comment:
   done



##########
gluten-core/src/main/java/org/apache/gluten/substrait/expression/WindowFunctionNode.java:
##########
@@ -80,20 +92,53 @@ private Expression.WindowFunction.Bound.Builder setBound(
         builder.setUnboundedFollowing(followingBuilder.build());
         break;
       default:
-        try {
-          Long offset = Long.valueOf(boundType);
+        if (boundType instanceof PreComputeRangeFrameBound) {
+          // Used only when backend is velox and frame type is RANGE.
+          if (frameType != "RANGE") {
+            throw new GlutenException(
+                "Only Range frame supports PreComputeRangeFrameBound, but got 
" + frameType);
+          }
+          ExpressionNode refNode =
+              ExpressionConverter.replaceWithExpressionTransformer(
+                      ((PreComputeRangeFrameBound) 
boundType).child().toAttribute(),
+                      
JavaConverters.asScalaIteratorConverter(originalInputAttributes.iterator())
+                          .asScala()
+                          .toSeq())
+                  .doTransform(new HashMap<String, Long>());
+          Long offset = Long.valueOf(boundType.eval(null).toString());
           if (offset < 0) {
-            Expression.WindowFunction.Bound.Preceding.Builder 
offsetPrecedingBuilder =
+            Expression.WindowFunction.Bound.Preceding.Builder 
refPrecedingBuilder =
                 Expression.WindowFunction.Bound.Preceding.newBuilder();
-            offsetPrecedingBuilder.setOffset(0 - offset);
-            builder.setPreceding(offsetPrecedingBuilder.build());
+            refPrecedingBuilder.setRef(refNode.toProtobuf());
+            builder.setPreceding(refPrecedingBuilder.build());
           } else {
-            Expression.WindowFunction.Bound.Following.Builder 
offsetFollowingBuilder =
+            Expression.WindowFunction.Bound.Following.Builder 
refFollowingBuilder =
                 Expression.WindowFunction.Bound.Following.newBuilder();
-            offsetFollowingBuilder.setOffset(offset);
-            builder.setFollowing(offsetFollowingBuilder.build());
+            refFollowingBuilder.setRef(refNode.toProtobuf());
+            builder.setFollowing(refFollowingBuilder.build());
+          }
+        } else if (boundType.foldable()) {
+          // Used when
+          // 1. Velox backend and frame type is ROW
+          // 2. Clickhouse backend
+          try {
+            Long offset = Long.valueOf(boundType.eval(null).toString());
+            if (offset < 0) {
+              Expression.WindowFunction.Bound.Preceding.Builder 
offsetPrecedingBuilder =
+                  Expression.WindowFunction.Bound.Preceding.newBuilder();
+              offsetPrecedingBuilder.setOffset(0 - offset);
+              builder.setPreceding(offsetPrecedingBuilder.build());
+            } else {
+              Expression.WindowFunction.Bound.Following.Builder 
offsetFollowingBuilder =
+                  Expression.WindowFunction.Bound.Following.newBuilder();
+              offsetFollowingBuilder.setOffset(offset);
+              builder.setFollowing(offsetFollowingBuilder.build());
+            }
+          } catch (NumberFormatException e) {

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

Reply via email to