You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2021/11/15 18:17:09 UTC

[GitHub] [flink] slinkydeveloper commented on a change in pull request #17786: [FLINK-24889][table] Flink SQL Client should print correсtly multisets

slinkydeveloper commented on a change in pull request #17786:
URL: https://github.com/apache/flink/pull/17786#discussion_r749572744



##########
File path: flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/MapAndMultisetToStringCastRule.java
##########
@@ -101,8 +109,16 @@ protected String generateCodeBlockInternal(
             String returnVariable,
             LogicalType inputLogicalType,
             LogicalType targetLogicalType) {
-        final LogicalType keyType = ((MapType) inputLogicalType).getKeyType();
-        final LogicalType valueType = ((MapType) inputLogicalType).getValueType();
+        boolean isMap = inputLogicalType instanceof MapType;

Review comment:
       No need to have a separate variable for this, just put it in the condition. Also please use the `is(LogicalTypeFamily)` method for checking if it's a map or not

##########
File path: flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/MapAndMultisetToStringCastRule.java
##########
@@ -34,25 +36,31 @@
 import static org.apache.flink.table.planner.functions.casting.CastRuleUtils.strLiteral;
 
 /** {@link LogicalTypeRoot#MAP} to {@link LogicalTypeFamily#CHARACTER_STRING} cast rule. */
-class MapToStringCastRule extends AbstractNullAwareCodeGeneratorCastRule<ArrayData, String> {
+class MapAndMultisetToStringCastRule
+        extends AbstractNullAwareCodeGeneratorCastRule<ArrayData, String> {
 
-    static final MapToStringCastRule INSTANCE = new MapToStringCastRule();
+    static final MapAndMultisetToStringCastRule INSTANCE = new MapAndMultisetToStringCastRule();
 
-    protected MapToStringCastRule(CastRulePredicate predicate) {
-        super(predicate);
-    }
-
-    private MapToStringCastRule() {
+    private MapAndMultisetToStringCastRule() {
         super(
                 CastRulePredicate.builder()
                         .predicate(
                                 (input, target) ->
-                                        input.is(LogicalTypeRoot.MAP)
-                                                && target.is(LogicalTypeFamily.CHARACTER_STRING)
-                                                && CastRuleProvider.exists(
-                                                        ((MapType) input).getKeyType(), target)
-                                                && CastRuleProvider.exists(
-                                                        ((MapType) input).getValueType(), target))
+                                        target.is(LogicalTypeFamily.CHARACTER_STRING)
+                                                && ((input.is(LogicalTypeRoot.MAP)
+                                                                && CastRuleProvider.exists(
+                                                                        ((MapType) input)
+                                                                                .getKeyType(),
+                                                                        target)
+                                                                && CastRuleProvider.exists(
+                                                                        ((MapType) input)
+                                                                                .getValueType(),
+                                                                        target))
+                                                        || (input.is(LogicalTypeRoot.MULTISET)
+                                                                && CastRuleProvider.exists(
+                                                                        ((MultisetType) input)
+                                                                                .getElementType(),
+                                                                        target))))

Review comment:
       Extract this in a separate method for clarity, like in `IdentityCastRule`




-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org