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


##########
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:
   if SAFE_CAST requires AS, it's not function syntax



##########
core/src/main/codegen/templates/Parser.jj:
##########
@@ -6008,6 +6008,18 @@ SqlNode BuiltinFunctionCall() :
         <RPAREN> {
             return SqlStdOperatorTable.CAST.createCall(s.end(this), args);
         }
+    |

Review Comment:
   could you combine with the logic for <CAST>?



##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2144,93 +2146,101 @@ private RexNode simplifySearch(RexCall call, 
RexUnknownAs unknownAs) {
     return call;
   }
 
-  private RexNode simplifyCast(RexCall e) {
+  private RexNode simplifyCast(RexCall e, boolean throwOnFailure) {

Review Comment:
   Does `throwOnFailure` relate to failure of this method - i.e. the effort to 
simplify - of failure of the cast at run time? You seem to have implemented the 
former, but I don't think that's what we need.



##########
core/src/main/java/org/apache/calcite/rex/RexBuilder.java:
##########
@@ -808,6 +904,29 @@ public RexNode makeAbstractCast(
         ImmutableList.of(exp));
   }
 
+  /**
+   * Creates a call to CAST or SAFE_CAST operator.
+   *
+   * @param type Type to cast to
+   * @param exp  Expression being cast
+   * @param kind  Either SqlKind.CAST or SqlKind.SAFE_CAST
+   * @return Call to CAST operator
+   */
+  public RexNode makeAbstractCast(
+      RelDataType type,
+      RexNode exp,
+      SqlKind kind) {
+    assert Arrays.asList(SqlKind.CAST, SqlKind.SAFE_CAST).contains(kind);
+    return kind == SqlKind.SAFE_CAST ? new RexCall(

Review Comment:
   can you re-organize this expression so that it is easier to see what's 
happening?



##########
core/src/main/java/org/apache/calcite/rex/RexExecutable.java:
##########
@@ -102,6 +102,38 @@ public void reduce(RexBuilder rexBuilder, List<RexNode> 
constExps,
     Hook.EXPRESSION_REDUCER.run(Pair.of(code, values));
   }
 
+  /**
+   * Same as {@link #reduce(RexBuilder, List, List)} but with a flag to throw 
on failure.
+   * <p>Catch the exception to return null for SAFE_ functions.</p>

Review Comment:
   looks like a lot of duplicate code



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