zoudan commented on code in PR #3093:
URL: https://github.com/apache/calcite/pull/3093#discussion_r1134834741


##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3481,6 +3481,7 @@ public static <E> Collection<E> 
multisetIntersectAll(Collection<E> c1,
     return result;
   }
 
+

Review Comment:
   remove empty line



##########
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Expressions.java:
##########
@@ -1164,6 +1164,7 @@ public static <F extends Function<?>> 
FunctionExpression<F> lambda(
     return new FunctionExpression<>(type, body, toList(parameters));
   }
 
+

Review Comment:
   remove this empty line



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##########
@@ -652,8 +714,8 @@ Expression translateCast(
       break;
     case TIMESTAMP_WITH_LOCAL_TIME_ZONE:
       convert =
-          RexImpTable.optimize2(operand,
-              Expressions.call(
+              RexImpTable.optimize2(

Review Comment:
   remove one tab?



##########
core/src/main/java/org/apache/calcite/sql/fun/SqlCastFunction.java:
##########
@@ -83,12 +84,25 @@ public class SqlCastFunction extends SqlFunction {
   //~ Constructors -----------------------------------------------------------
 
   public SqlCastFunction() {
-    super("CAST",
-        SqlKind.CAST,
+    this(SqlKind.CAST);
+  }
+
+  private SqlCastFunction(SqlKind kind) {

Review Comment:
   How about just keep this constructor and remove `withKind`



##########
core/src/main/java/org/apache/calcite/sql/fun/SqlCastFunction.java:
##########
@@ -168,7 +182,7 @@ public SqlCastFunction() {
   }
 
   @Override public SqlSyntax getSyntax() {
-    return SqlSyntax.SPECIAL;
+    return this.kind == SqlKind.CAST ? SqlSyntax.SPECIAL : SqlSyntax.FUNCTION;

Review Comment:
   +1, SAFE_CAST is a special SqlSyntax



##########
core/src/main/java/org/apache/calcite/sql/SqlKind.java:
##########
@@ -639,6 +639,10 @@ public enum SqlKind {
    */
   CAST,
 
+  /**
+   * The {@code SAFE_CAST} function. */

Review Comment:
   It is better to add description for what is the difference between 
`SAFE_CAST` and `CAST`



##########
babel/src/test/resources/sql/big-query.iq:
##########


Review Comment:
   shall we also add some tests in `SqlOperatorTest`?



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##########
@@ -267,10 +268,39 @@ Expression translate(RexNode expr, RexImpTable.NullAs 
nullAs,
     return nullAs.handle(translated);
   }
 
+  /**
+   * Used for safe operators that return null if an exception is thrown.
+   */
+  Expression expressionHandlingSafe(Expression body, boolean safe) {
+    if (safe) {
+      return safeExpression(body);
+    } else {
+      return body;
+    }
+  }
+
+  public static Expression safeExpression(Expression body) {

Review Comment:
   could be private?



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java:
##########
@@ -267,10 +268,39 @@ Expression translate(RexNode expr, RexImpTable.NullAs 
nullAs,
     return nullAs.handle(translated);
   }
 
+  /**
+   * Used for safe operators that return null if an exception is thrown.
+   */
+  Expression expressionHandlingSafe(Expression body, boolean safe) {

Review Comment:
   could be private?



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

Reply via email to