twalthr commented on a change in pull request #18063:
URL: https://github.com/apache/flink/pull/18063#discussion_r767637863



##########
File path: 
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/CharVarCharTrimPadCastRule.java
##########
@@ -93,19 +92,18 @@ protected String generateCodeBlockInternal(
         CastRule<?, ?> castRule =
                 CastRuleProvider.resolve(inputLogicalType, 
VarCharType.STRING_TYPE);
 
-        // Only used for non-Constructed types - for constructed, the 
trimming/padding is applied
-        // on each individual rule, i.e.: ArrayToStringCastRule
+        // Only used for non-Constructed types - for constructed type and RAW, 
the trimming/padding
+        // is applied on each individual rule, i.e.: ArrayToStringCastRule, 
RawToStringCastRule
         if (castRule instanceof ExpressionCodeGeneratorCastRule) {
             @SuppressWarnings("rawtypes")
             final String stringExpr =
                     ((ExpressionCodeGeneratorCastRule) castRule)
                             .generateExpression(
                                     context, inputTerm, inputLogicalType, 
targetLogicalType);
 
-            CastRuleUtils.CodeWriter writer = new CastRuleUtils.CodeWriter();
+            final CastRuleUtils.CodeWriter writer = new 
CastRuleUtils.CodeWriter();
             if (context.legacyBehaviour()
-                    || !(shouldPossiblyTrim(precision)
-                            || shouldPossiblyPad(targetLogicalType, 
precision))) {
+                    || !(shouldTrim(precision) || shouldPad(targetLogicalType, 
precision))) {

Review comment:
       nit: `precision` -> `length`

##########
File path: 
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/CharVarCharTrimPadCastRule.java
##########
@@ -43,19 +43,18 @@
  * the one specified by the precision of the target {@link 
LogicalTypeRoot#CHAR} or {@link
  * LogicalTypeRoot#VARCHAR} type.
  */
-class CharacterFamilyTrimmingAndPaddingCastRule
-        extends AbstractNullAwareCodeGeneratorCastRule<StringData, StringData> 
{
+class CharVarCharTrimPadCastRule
+        extends AbstractNullAwareCodeGeneratorCastRule<Object, StringData> {
 
-    static final CharacterFamilyTrimmingAndPaddingCastRule INSTANCE =
-            new CharacterFamilyTrimmingAndPaddingCastRule();
+    static final CharVarCharTrimPadCastRule INSTANCE = new 
CharVarCharTrimPadCastRule();
 
-    private CharacterFamilyTrimmingAndPaddingCastRule() {
+    private CharVarCharTrimPadCastRule() {
         super(
                 CastRulePredicate.builder()
                         .predicate(
                                 (inputType, targetType) ->
                                         
targetType.is(LogicalTypeFamily.CHARACTER_STRING)
-                                                && !isStringType(targetType))
+                                                && 
!targetType.equals(STRING_TYPE))

Review comment:
       can this also be converted to a non-predicate now?

##########
File path: 
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/ArrayToStringCastRule.java
##########
@@ -144,11 +144,10 @@ protected String generateCodeBlockInternal(
                                                     innerInputType.copy(false),
                                                     STRING_TYPE);
 
-                                    if (!context.legacyBehaviour()
-                                            && shouldPossiblyTrim(precision)) {
+                                    if (!context.legacyBehaviour() && 
shouldTrim(length)) {
                                         // Break if the target precision is 
already exceeded
                                         loopBodyWriter.ifStmt(
-                                                
lengthExceedsPrecision(builderTerm, precision),
+                                                
lengthExceedsPrecision(builderTerm, length),

Review comment:
       also find a better name for this method?

##########
File path: 
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/CharVarCharTrimPadCastRule.java
##########
@@ -168,15 +170,15 @@ static String precisionExceedsLength(String strTerm, int 
precision) {
         return methodCall(strTerm, "length") + " < " + precision;
     }
 
-    static boolean shouldPossiblyTrim(int precision) {
+    static boolean shouldTrim(int precision) {
         return precision < VarCharType.MAX_LENGTH;
     }
 
-    static boolean shouldPossiblyPad(LogicalType targetType, int precision) {
+    static boolean shouldPad(LogicalType targetType, int precision) {
         return targetType.is(LogicalTypeRoot.CHAR) && precision < 
VarCharType.MAX_LENGTH;
     }
 
-    public static CastRuleUtils.CodeWriter appendAndTrimStringIfNeeded(
+    public static CastRuleUtils.CodeWriter padAndTrimStringIfNeeded(

Review comment:
       default scope?




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