[fixup] PR feedback

Signed-off-by: slinkydeveloper <francescoguard@gmail.com>
apache · Nov 5, 2021 · 7dfe3c6 · 7dfe3c6
1 parent 5b8ef0d
commit 7dfe3c6
Show file tree

Hide file tree

Showing 16 changed files with 48 additions and 64 deletions.
diff --git a/...nner/src/main/java/org/apache/flink/table/planner/functions/casting/CastRuleProvider.java b/...nner/src/main/java/org/apache/flink/table/planner/functions/casting/CastRuleProvider.java
@@ -28,7 +28,7 @@
 import org.apache.flink.table.planner.functions.casting.rules.IdentityCastRule;
 import org.apache.flink.table.planner.functions.casting.rules.IntervalToStringCastRule;
 import org.apache.flink.table.planner.functions.casting.rules.MapToStringCastRule;
-import org.apache.flink.table.planner.functions.casting.rules.NumberToStringCastRule;
+import org.apache.flink.table.planner.functions.casting.rules.NumericToStringCastRule;
 import org.apache.flink.table.planner.functions.casting.rules.RawToStringCastRule;
 import org.apache.flink.table.planner.functions.casting.rules.RowToStringCastRule;
 import org.apache.flink.table.planner.functions.casting.rules.TimeToStringCastRule;
@@ -58,7 +58,7 @@ public class CastRuleProvider {
                 // Numeric rules
                 .addRule(UpcastToBigIntCastRule.INSTANCE)
                 // To string rules
-                .addRule(NumberToStringCastRule.INSTANCE)
+                .addRule(NumericToStringCastRule.INSTANCE)
                 .addRule(BooleanToStringCastRule.INSTANCE)
                 .addRule(BinaryToStringCastRule.INSTANCE)
                 .addRule(TimestampToStringCastRule.INSTANCE)

diff --git a/...apache/flink/table/planner/functions/casting/rules/AbstractCharacterFamilyTargetRule.java b/...apache/flink/table/planner/functions/casting/rules/AbstractCharacterFamilyTargetRule.java
@@ -26,7 +26,6 @@
 import org.apache.flink.table.types.logical.LogicalTypeFamily;
 
 import static org.apache.flink.table.planner.codegen.calls.BuiltInMethods.BINARY_STRING_DATA_FROM_STRING;
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.functionCall;
 
 /**
  * Base class for cast rules converting to {@link LogicalTypeFamily#CHARACTER_STRING} with code
@@ -55,6 +54,6 @@ String generateExpression(
         final String stringExpr =
                 generateStringExpression(context, inputTerm, inputLogicalType, targetLogicalType);
 
-        return functionCall(BINARY_STRING_DATA_FROM_STRING(), stringExpr);
+        return CastRuleUtils.staticCall(BINARY_STRING_DATA_FROM_STRING(), stringExpr);
     }
 }
diff --git a/...e/flink/table/planner/functions/casting/rules/AbstractNullAwareCodeGeneratorCastRule.java b/...e/flink/table/planner/functions/casting/rules/AbstractNullAwareCodeGeneratorCastRule.java
@@ -81,11 +81,11 @@ public CastCodeBlock generateCodeBlock(
         if (isResultNullable) {
             writer.ifStmt(
                     "!" + nullTerm,
-                    thenWriter -> thenWriter.append(castCodeBlock),
+                    thenWriter -> thenWriter.appendBlock(castCodeBlock),
                     elseWriter ->
                             elseWriter.assignStmt(returnTerm, primitiveDefaultValue(targetType)));
         } else {
-            writer.append(castCodeBlock).append("\n");
+            writer.appendBlock(castCodeBlock);
         }
 
         return new CastCodeBlock(writer.toString(), returnTerm, nullTerm);

diff --git a/...ain/java/org/apache/flink/table/planner/functions/casting/rules/ArrayToArrayCastRule.java b/...ain/java/org/apache/flink/table/planner/functions/casting/rules/ArrayToArrayCastRule.java
@@ -33,7 +33,6 @@
 import static org.apache.flink.table.planner.codegen.CodeGenUtils.newName;
 import static org.apache.flink.table.planner.codegen.CodeGenUtils.rowFieldReadAccess;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.constructorCall;
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.functionCall;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.methodCall;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.newArray;
 
@@ -97,7 +96,7 @@ protected String generateCodeBlockInternal(
 
                             if (innerTargetType.isNullable()) {
                                 loopWriter.ifStmt(
-                                        "!" + functionCall(inputTerm + ".isNullAt", index),
+                                        "!" + methodCall(inputTerm, "isNullAt", index),
                                         thenWriter ->
                                                 thenWriter
                                                         .append(codeBlock)

diff --git a/...in/java/org/apache/flink/table/planner/functions/casting/rules/ArrayToStringCastRule.java b/...in/java/org/apache/flink/table/planner/functions/casting/rules/ArrayToStringCastRule.java
@@ -34,7 +34,6 @@
 import static org.apache.flink.table.planner.codegen.calls.BuiltInMethods.BINARY_STRING_DATA_FROM_STRING;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.NULL_STR_LITERAL;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.constructorCall;
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.functionCall;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.methodCall;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.strLiteral;
 
@@ -162,7 +161,7 @@ protected String generateCodeBlockInternal(
                 // Assign the result value
                 .assignStmt(
                         returnVariable,
-                        functionCall(
+                        CastRuleUtils.staticCall(
                                 BINARY_STRING_DATA_FROM_STRING(),
                                 methodCall(builderTerm, "toString")))
                 .toString();

diff --git a/.../java/org/apache/flink/table/planner/functions/casting/rules/BooleanToStringCastRule.java b/.../java/org/apache/flink/table/planner/functions/casting/rules/BooleanToStringCastRule.java
@@ -24,7 +24,7 @@
 import org.apache.flink.table.types.logical.LogicalTypeFamily;
 import org.apache.flink.table.types.logical.LogicalTypeRoot;
 
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.strLiteral;
+import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.EMPTY_STR_LITERAL;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.stringConcat;
 
 /** {@link LogicalTypeRoot#BOOLEAN} to {@link LogicalTypeFamily#CHARACTER_STRING} cast rule. */
@@ -46,6 +46,6 @@ public String generateStringExpression(
             String inputTerm,
             LogicalType inputLogicalType,
             LogicalType targetLogicalType) {
-        return stringConcat(strLiteral(), inputTerm);
+        return stringConcat(EMPTY_STR_LITERAL, inputTerm);
     }
 }
diff --git a/...r/src/main/java/org/apache/flink/table/planner/functions/casting/rules/CastRuleUtils.java b/...r/src/main/java/org/apache/flink/table/planner/functions/casting/rules/CastRuleUtils.java
@@ -22,8 +22,7 @@
 import org.apache.flink.table.planner.functions.casting.CastCodeBlock;
 import org.apache.flink.table.planner.functions.casting.CastRule;
 import org.apache.flink.table.types.logical.LogicalType;
-
-import org.apache.commons.lang3.StringEscapeUtils;
+import org.apache.flink.table.utils.EncodingUtils;
 
 import java.lang.reflect.Method;
 import java.util.Arrays;
@@ -40,15 +39,9 @@
 final class CastRuleUtils {
 
     static final String NULL_STR_LITERAL = strLiteral("NULL");
+    static final String EMPTY_STR_LITERAL = "\"\"";
 
-    static String functionCall(String functionName, Object... args) {
-        return functionName
-                + "("
-                + Arrays.stream(args).map(Object::toString).collect(Collectors.joining(", "))
-                + ")";
-    }
-
-    static String functionCall(Method staticMethod, Object... args) {
+    static String staticCall(Method staticMethod, Object... args) {
         return functionCall(CodeGenUtils.qualifyMethod(staticMethod), args);
     }
 
@@ -60,6 +53,13 @@ static String methodCall(String instanceTerm, String methodName, Object... args)
         return functionCall(instanceTerm + "." + methodName, args);
     }
 
+    private static String functionCall(String functionName, Object... args) {
+        return functionName
+                + "("
+                + Arrays.stream(args).map(Object::toString).collect(Collectors.joining(", "))
+                + ")";
+    }
+
     static String newArray(String innerType, String arraySize) {
         return "new " + innerType + "[" + arraySize + "]";
     }
@@ -76,12 +76,8 @@ static String ternaryOperator(String condition, String ifTrue, String ifFalse) {
         return "((" + condition + ") ? (" + ifTrue + ") : (" + ifFalse + "))";
     }
 
-    static String strLiteral() {
-        return "\"\"";
-    }
-
     static String strLiteral(String str) {
-        return "\"" + StringEscapeUtils.escapeJava(str) + "\"";
+        return "\"" + EncodingUtils.escapeJava(str) + "\"";
     }
 
     static final class CodeWriter {
@@ -178,7 +174,7 @@ public CodeWriter append(CastCodeBlock codeBlock) {
             return this;
         }
 
-        public CodeWriter append(String codeBlock) {
+        public CodeWriter appendBlock(String codeBlock) {
             builder.append(codeBlock);
             return this;
         }

diff --git a/...ain/java/org/apache/flink/table/planner/functions/casting/rules/DateToStringCastRule.java b/...ain/java/org/apache/flink/table/planner/functions/casting/rules/DateToStringCastRule.java
@@ -25,7 +25,7 @@
 import org.apache.flink.table.types.logical.LogicalTypeRoot;
 
 import static org.apache.flink.table.planner.codegen.calls.BuiltInMethods.UNIX_DATE_TO_STRING;
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.functionCall;
+import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.staticCall;
 
 /** {@link LogicalTypeRoot#DATE} to {@link LogicalTypeFamily#CHARACTER_STRING} cast rule. */
 public class DateToStringCastRule extends AbstractCharacterFamilyTargetRule<Long> {
@@ -46,6 +46,6 @@ public String generateStringExpression(
             String inputTerm,
             LogicalType inputLogicalType,
             LogicalType targetLogicalType) {
-        return functionCall(UNIX_DATE_TO_STRING(), inputTerm);
+        return staticCall(UNIX_DATE_TO_STRING(), inputTerm);
     }
 }
diff --git a/...rc/main/java/org/apache/flink/table/planner/functions/casting/rules/IdentityCastRule.java b/...rc/main/java/org/apache/flink/table/planner/functions/casting/rules/IdentityCastRule.java
@@ -24,6 +24,7 @@
 import org.apache.flink.table.planner.functions.casting.CodeGeneratorCastRule;
 import org.apache.flink.table.types.logical.LogicalType;
 import org.apache.flink.table.types.logical.LogicalTypeFamily;
+import org.apache.flink.table.types.logical.utils.LogicalTypeCasts;
 
 /**
  * Identity cast rule. For more details on when the rule is applied, check {@link
@@ -48,7 +49,7 @@ private static boolean isIdentityCast(
         }
 
         // Identity cast applies if the two types are equals, except nullability
-        return inputLogicalType.copy(true).equals(targetLogicalType.copy(true));
+        return LogicalTypeCasts.supportsAvoidingCast(inputLogicalType, targetLogicalType);
     }
 
     @Override

diff --git a/...java/org/apache/flink/table/planner/functions/casting/rules/IntervalToStringCastRule.java b/...java/org/apache/flink/table/planner/functions/casting/rules/IntervalToStringCastRule.java
@@ -28,7 +28,7 @@
 
 import static org.apache.flink.table.planner.codegen.calls.BuiltInMethods.INTERVAL_DAY_TIME_TO_STRING;
 import static org.apache.flink.table.planner.codegen.calls.BuiltInMethods.INTERVAL_YEAR_MONTH_TO_STRING;
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.functionCall;
+import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.staticCall;
 
 /** {@link LogicalTypeFamily#INTERVAL} to {@link LogicalTypeFamily#CHARACTER_STRING} cast rule. */
 public class IntervalToStringCastRule extends AbstractCharacterFamilyTargetRule<Object> {
@@ -53,6 +53,6 @@ public String generateStringExpression(
                 inputLogicalType.is(LogicalTypeRoot.INTERVAL_YEAR_MONTH)
                         ? INTERVAL_YEAR_MONTH_TO_STRING()
                         : INTERVAL_DAY_TIME_TO_STRING();
-        return functionCall(method, inputTerm);
+        return staticCall(method, inputTerm);
     }
 }
diff --git a/...main/java/org/apache/flink/table/planner/functions/casting/rules/MapToStringCastRule.java b/...main/java/org/apache/flink/table/planner/functions/casting/rules/MapToStringCastRule.java
@@ -34,7 +34,6 @@
 import static org.apache.flink.table.planner.codegen.calls.BuiltInMethods.BINARY_STRING_DATA_FROM_STRING;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.NULL_STR_LITERAL;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.constructorCall;
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.functionCall;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.methodCall;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.strLiteral;
 
@@ -220,7 +219,7 @@ protected String generateCodeBlockInternal(
                 // Assign the result value
                 .assignStmt(
                         returnVariable,
-                        functionCall(
+                        CastRuleUtils.staticCall(
                                 BINARY_STRING_DATA_FROM_STRING(),
                                 methodCall(builderTerm, "toString")))
                 .toString();

diff --git a/...casting/rules/NumberToStringCastRule.java → ...asting/rules/NumericToStringCastRule.java b/...casting/rules/NumberToStringCastRule.java → ...asting/rules/NumericToStringCastRule.java
@@ -23,15 +23,15 @@
 import org.apache.flink.table.types.logical.LogicalType;
 import org.apache.flink.table.types.logical.LogicalTypeFamily;
 
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.strLiteral;
+import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.EMPTY_STR_LITERAL;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.stringConcat;
 
 /** {@link LogicalTypeFamily#NUMERIC} to {@link LogicalTypeFamily#CHARACTER_STRING} cast rule. */
-public class NumberToStringCastRule extends AbstractCharacterFamilyTargetRule<Object> {
+public class NumericToStringCastRule extends AbstractCharacterFamilyTargetRule<Object> {
 
-    public static final NumberToStringCastRule INSTANCE = new NumberToStringCastRule();
+    public static final NumericToStringCastRule INSTANCE = new NumericToStringCastRule();
 
-    private NumberToStringCastRule() {
+    private NumericToStringCastRule() {
         super(
                 CastRulePredicate.builder()
                         .input(LogicalTypeFamily.NUMERIC)
@@ -45,6 +45,6 @@ public String generateStringExpression(
             String inputTerm,
             LogicalType inputLogicalType,
             LogicalType targetLogicalType) {
-        return stringConcat(strLiteral(), inputTerm);
+        return stringConcat(EMPTY_STR_LITERAL, inputTerm);
     }
 }
diff --git a/...main/java/org/apache/flink/table/planner/functions/casting/rules/RawToStringCastRule.java b/...main/java/org/apache/flink/table/planner/functions/casting/rules/RawToStringCastRule.java
@@ -26,7 +26,6 @@
 
 import static org.apache.flink.table.codesplit.CodeSplitUtil.newName;
 import static org.apache.flink.table.planner.codegen.calls.BuiltInMethods.BINARY_STRING_DATA_FROM_STRING;
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.functionCall;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.methodCall;
 
 /** {@link LogicalTypeRoot#RAW} to {@link LogicalTypeFamily#CHARACTER_STRING} cast rule. */
@@ -62,7 +61,7 @@ protected String generateCodeBlockInternal(
                         thenWriter ->
                                 thenWriter.assignStmt(
                                         returnVariable,
-                                        functionCall(
+                                        CastRuleUtils.staticCall(
                                                 BINARY_STRING_DATA_FROM_STRING(),
                                                 methodCall(deserializedObjTerm, "toString"))),
                         elseWriter -> elseWriter.assignStmt(returnVariable, "null"))

diff --git a/...main/java/org/apache/flink/table/planner/functions/casting/rules/RowToStringCastRule.java b/...main/java/org/apache/flink/table/planner/functions/casting/rules/RowToStringCastRule.java
@@ -37,7 +37,6 @@
 import static org.apache.flink.table.planner.codegen.calls.BuiltInMethods.BINARY_STRING_DATA_FROM_STRING;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.NULL_STR_LITERAL;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.constructorCall;
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.functionCall;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.methodCall;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.strLiteral;
 
@@ -47,22 +46,16 @@ public class RowToStringCastRule extends AbstractNullAwareCodeGeneratorCastRule<
     public static final RowToStringCastRule INSTANCE = new RowToStringCastRule();
 
     private RowToStringCastRule() {
-        super(
-                CastRulePredicate.builder()
-                        .predicate(
-                                (input, target) ->
-                                        input.is(LogicalTypeRoot.ROW)
-                                                && target.is(LogicalTypeFamily.CHARACTER_STRING)
-                                                && ((RowType) input)
-                                                        .getFields().stream()
-                                                                .allMatch(
-                                                                        field ->
-                                                                                CastRuleProvider
-                                                                                        .exists(
-                                                                                                field
-                                                                                                        .getType(),
-                                                                                                target)))
-                        .build());
+        super(CastRulePredicate.builder().predicate(RowToStringCastRule::matches).build());
+    }
+
+    private static boolean matches(LogicalType input, LogicalType target) {
+        return input.is(LogicalTypeRoot.ROW)
+                && target.is(LogicalTypeFamily.CHARACTER_STRING)
+                && ((RowType) input)
+                        .getFields().stream()
+                                .allMatch(
+                                        field -> CastRuleProvider.exists(field.getType(), target));
     }
 
     /* Example generated code for ROW<`f0` INT, `f1` STRING>:
@@ -172,7 +165,7 @@ protected String generateCodeBlockInternal(
                 // Assign the result value
                 .assignStmt(
                         returnVariable,
-                        functionCall(
+                        CastRuleUtils.staticCall(
                                 BINARY_STRING_DATA_FROM_STRING(),
                                 methodCall(builderTerm, "toString")));
 

diff --git a/...ain/java/org/apache/flink/table/planner/functions/casting/rules/TimeToStringCastRule.java b/...ain/java/org/apache/flink/table/planner/functions/casting/rules/TimeToStringCastRule.java
@@ -26,7 +26,6 @@
 import org.apache.flink.table.types.logical.utils.LogicalTypeChecks;
 
 import static org.apache.flink.table.planner.codegen.calls.BuiltInMethods.UNIX_TIME_TO_STRING;
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.functionCall;
 
 /**
  * {@link LogicalTypeRoot#TIME_WITHOUT_TIME_ZONE} to {@link LogicalTypeFamily#CHARACTER_STRING} cast
@@ -50,7 +49,7 @@ public String generateStringExpression(
             String inputTerm,
             LogicalType inputLogicalType,
             LogicalType targetLogicalType) {
-        return functionCall(
+        return CastRuleUtils.staticCall(
                 UNIX_TIME_TO_STRING(), inputTerm, LogicalTypeChecks.getPrecision(inputLogicalType));
     }
 }
diff --git a/...ava/org/apache/flink/table/planner/functions/casting/rules/TimestampToStringCastRule.java b/...ava/org/apache/flink/table/planner/functions/casting/rules/TimestampToStringCastRule.java
@@ -31,7 +31,7 @@
 
 import static org.apache.flink.table.planner.codegen.calls.BuiltInMethods.TIMESTAMP_TO_STRING_TIME_ZONE;
 import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.accessStaticField;
-import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.functionCall;
+import static org.apache.flink.table.planner.functions.casting.rules.CastRuleUtils.staticCall;
 
 /** {@link LogicalTypeFamily#TIMESTAMP} to {@link LogicalTypeFamily#CHARACTER_STRING} cast rule. */
 @Internal
@@ -59,6 +59,6 @@ public String generateStringExpression(
                         : accessStaticField(DateTimeUtils.class, "UTC_ZONE");
         final int precision = LogicalTypeChecks.getPrecision(inputLogicalType);
 
-        return functionCall(TIMESTAMP_TO_STRING_TIME_ZONE(), inputTerm, zoneId, precision);
+        return staticCall(TIMESTAMP_TO_STRING_TIME_ZONE(), inputTerm, zoneId, precision);
     }
 }