You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "liuyongvs (via GitHub)" <gi...@apache.org> on 2023/03/31 05:37:14 UTC

[GitHub] [flink] liuyongvs opened a new pull request, #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

liuyongvs opened a new pull request, #22312:
URL: https://github.com/apache/flink/pull/22312

   - What is the purpose of the change
   This is an implementation of MAP_ENTRIES
   
   - Brief change log
   MAP_ENTRIES for Table API and SQL
       
     ```
   map_entries(map) - Returns an unordered array of all entries in the given map.
   
   Syntax:
   map_entries(map)
   
   Arguments:
   map: An MAP to be handled.
   
   Returns:
   
   Returns an unordered array of all entries in the given map. Returns null if the argument is null
   
   > SELECT map_entries(map[1, 'a', 2, 'b']);
    [(1,"a"),(2,"b")]
     ```
   
   
   See also
   presto https://prestodb.io/docs/current/functions/map.html
   spark https://spark.apache.org/docs/latest/api/sql/index.html#map_entries
   
   - Verifying this change
   This change added tests in MapFunctionITCase.
   
   - Does this pull request potentially affect one of the following parts:
   Dependencies (does it add or upgrade a dependency): ( no)
   The public API, i.e., is any changed class annotated with @Public(Evolving): (yes )
   The serializers: (no)
   The runtime per-record code paths (performance sensitive): ( no)
   Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: ( no)
   The S3 file system connector: ( no)
   - Documentation
   Does this pull request introduce a new feature? (yes)
   If yes, how is the feature documented? (docs)


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


[GitHub] [flink] liuyongvs commented on a diff in pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on code in PR #22312:
URL: https://github.com/apache/flink/pull/22312#discussion_r1201400315


##########
docs/data/sql_functions.yml:
##########
@@ -646,6 +646,9 @@ collection:
   - sql: MAP_VALUES(map)
     table: MAP.mapValues()
     description: Returns the values of the map as array. No order guaranteed.
+  - sql: MAP_ENTRIES(map)
+    table: MAP.mapEntries()
+    description: Returns an unordered array of all entries in the given map. No order guaranteed.

Review Comment:
   keys are not sorted, i will remove it @snuyanzin 



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


[GitHub] [flink] liuyongvs commented on pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22312:
URL: https://github.com/apache/flink/pull/22312#issuecomment-1558388581

   rebase and fix your reviews @snuyanzin 


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


[GitHub] [flink] liuyongvs commented on pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22312:
URL: https://github.com/apache/flink/pull/22312#issuecomment-1575008696

   hi @snuyanzin do you have time look again?


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


[GitHub] [flink] liuyongvs commented on pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22312:
URL: https://github.com/apache/flink/pull/22312#issuecomment-1558666079

   hi @snuyanzin ci passed


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


[GitHub] [flink] snuyanzin commented on a diff in pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #22312:
URL: https://github.com/apache/flink/pull/22312#discussion_r1200534874


##########
flink-python/pyflink/table/expression.py:
##########
@@ -1519,10 +1519,19 @@ def map_values(self) -> 'Expression':
         """
         Returns the values of the map as an array. No order guaranteed.
 
-        .. seealso:: :py:attr:`~Expression.map_keys`
+        .. seealso:: :py:attr:`~Expression.map_values`
         """
         return _unary_op("mapValues")(self)
 
+    @property
+    def map_entries(self) -> 'Expression':
+        """
+        Returns an unordered array of all entries in the given map. No order guaranteed.

Review Comment:
   same about `unordered`
   



##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/BaseExpressions.java:
##########
@@ -1396,6 +1397,11 @@ public OutType mapValues() {
         return toApiSpecificExpression(unresolvedCall(MAP_VALUES, toExpr()));
     }
 
+    /** Returns an unordered array of all entries in the given map. */

Review Comment:
   same about `unordered`



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


[GitHub] [flink] snuyanzin commented on a diff in pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #22312:
URL: https://github.com/apache/flink/pull/22312#discussion_r1200534041


##########
docs/data/sql_functions.yml:
##########
@@ -646,6 +646,9 @@ collection:
   - sql: MAP_VALUES(map)
     table: MAP.mapValues()
     description: Returns the values of the map as array. No order guaranteed.
+  - sql: MAP_ENTRIES(map)
+    table: MAP.mapEntries()
+    description: Returns an unordered array of all entries in the given map. No order guaranteed.

Review Comment:
   what means `unordered` here?
   



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


[GitHub] [flink] liuyongvs commented on pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22312:
URL: https://github.com/apache/flink/pull/22312#issuecomment-1557131089

   hi @snuyanzin do you have time to review it?


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


[GitHub] [flink] luoyuxia commented on a diff in pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia commented on code in PR #22312:
URL: https://github.com/apache/flink/pull/22312#discussion_r1221074956


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/SpecificTypeStrategies.java:
##########
@@ -89,6 +95,49 @@ public final class SpecificTypeStrategies {
     /** See {@link ToTimestampLtzTypeStrategy}. */
     public static final TypeStrategy TO_TIMESTAMP_LTZ = new ToTimestampLtzTypeStrategy();
 
+    /** Type strategy specific for {@link BuiltInFunctionDefinitions#MAP_KEYS}. */
+    public static final TypeStrategy MAP_KEYS =
+            callContext ->
+                    Optional.of(
+                            DataTypes.ARRAY(
+                                    ((KeyValueDataType) callContext.getArgumentDataTypes().get(0))
+                                            .getKeyDataType()));
+
+    /** Type strategy specific for {@link BuiltInFunctionDefinitions#MAP_VALUES}. */
+    public static final TypeStrategy MAP_VALUES =
+            callContext ->
+                    Optional.of(
+                            DataTypes.ARRAY(
+                                    ((KeyValueDataType) callContext.getArgumentDataTypes().get(0))
+                                            .getValueDataType()));
+
+    /** Type strategy specific for {@link BuiltInFunctionDefinitions#MAP_ENTRIES}. */
+    public static final TypeStrategy MAP_ENTRIES =
+            callContext ->
+                    Optional.of(
+                            DataTypes.ARRAY(
+                                    DataTypes.ROW(

Review Comment:
   Do we need to specify the name for  each field of the `Row` type? Btw, seems spark use
   ```
         StructType(
           StructField("key", childDataType.keyType, false) ::
           StructField("value", childDataType.valueType, childDataType.valueContainsNull) ::
           Nil)
   ````



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


[GitHub] [flink] liuyongvs commented on a diff in pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on code in PR #22312:
URL: https://github.com/apache/flink/pull/22312#discussion_r1221171695


##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/SpecificTypeStrategies.java:
##########
@@ -89,6 +95,49 @@ public final class SpecificTypeStrategies {
     /** See {@link ToTimestampLtzTypeStrategy}. */
     public static final TypeStrategy TO_TIMESTAMP_LTZ = new ToTimestampLtzTypeStrategy();
 
+    /** Type strategy specific for {@link BuiltInFunctionDefinitions#MAP_KEYS}. */
+    public static final TypeStrategy MAP_KEYS =
+            callContext ->
+                    Optional.of(
+                            DataTypes.ARRAY(
+                                    ((KeyValueDataType) callContext.getArgumentDataTypes().get(0))
+                                            .getKeyDataType()));
+
+    /** Type strategy specific for {@link BuiltInFunctionDefinitions#MAP_VALUES}. */
+    public static final TypeStrategy MAP_VALUES =
+            callContext ->
+                    Optional.of(
+                            DataTypes.ARRAY(
+                                    ((KeyValueDataType) callContext.getArgumentDataTypes().get(0))
+                                            .getValueDataType()));
+
+    /** Type strategy specific for {@link BuiltInFunctionDefinitions#MAP_ENTRIES}. */
+    public static final TypeStrategy MAP_ENTRIES =
+            callContext ->
+                    Optional.of(
+                            DataTypes.ARRAY(
+                                    DataTypes.ROW(

Review Comment:
   added @luoyuxia 



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


[GitHub] [flink] flinkbot commented on pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22312:
URL: https://github.com/apache/flink/pull/22312#issuecomment-1491329647

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "13959f8e4ede00ec672129170259b32761ffe6e4",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "13959f8e4ede00ec672129170259b32761ffe6e4",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 13959f8e4ede00ec672129170259b32761ffe6e4 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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


[GitHub] [flink] snuyanzin commented on a diff in pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #22312:
URL: https://github.com/apache/flink/pull/22312#discussion_r1200534431


##########
flink-python/docs/reference/pyflink.table/expressions.rst:
##########
@@ -231,6 +231,7 @@ advanced type helper functions
     Expression.array_remove
     Expression.map_keys
     Expression.map_values
+    Expression.map_entries

Review Comment:
   it's better to have abc order



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


[GitHub] [flink] liuyongvs commented on pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22312:
URL: https://github.com/apache/flink/pull/22312#issuecomment-1568072002

   hi @snuyanzin 


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


[GitHub] [flink] luoyuxia merged pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "luoyuxia (via GitHub)" <gi...@apache.org>.
luoyuxia merged PR #22312:
URL: https://github.com/apache/flink/pull/22312


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


[GitHub] [flink] liuyongvs commented on pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22312:
URL: https://github.com/apache/flink/pull/22312#issuecomment-1562585279

   hi @snuyanzin do you have time look again?


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


[GitHub] [flink] liuyongvs commented on pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22312:
URL: https://github.com/apache/flink/pull/22312#issuecomment-1491722243

   hi @snuyanzin do you have time to review it?


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


[GitHub] [flink] liuyongvs commented on pull request #22312: [FLINK-31677][table] Add built-in MAP_ENTRIES function.

Posted by "liuyongvs (via GitHub)" <gi...@apache.org>.
liuyongvs commented on PR #22312:
URL: https://github.com/apache/flink/pull/22312#issuecomment-1549370329

   hi @dawidwys could you have a look this ,it is long time no one to review and merge?


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