You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "zhangjun0x01 (via GitHub)" <gi...@apache.org> on 2023/04/26 01:54:56 UTC

[GitHub] [incubator-paimon] zhangjun0x01 opened a new pull request, #1026: [flink]add spatial type for mysql cdc action

zhangjun0x01 opened a new pull request, #1026:
URL: https://github.com/apache/incubator-paimon/pull/1026

   *(Please specify the module before the PR name: [core] ... or [flink] ...)*
   
   ### Purpose
   
   **issue https://github.com/apache/incubator-paimon/issues/966 **
   
   ### Tests
   
   *org.apache.paimon.flink.action.cdc.mysql.MySqlSyncTableActionITCase*
   
   ### API and Format 
   
   *(Does this change affect API or storage format)*
   
   ### Documentation
   
   *(Does this change introduce a new feature)*
   


-- 
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@paimon.apache.org

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


[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1026: [flink]add spatial type for mysql cdc action

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1026:
URL: https://github.com/apache/incubator-paimon/pull/1026#discussion_r1177546599


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/action/cdc/mysql/MySqlDebeziumJsonEventParser.java:
##########
@@ -200,18 +203,27 @@ public List<CdcRecord> getRecords() {
     }
 
     private Map<String, String> extractRow(JsonNode recordRow) {
-        Map<String, String> recordMap =
-                objectMapper.convertValue(recordRow, new TypeReference<Map<String, String>>() {});
-        if (recordMap == null) {
+
+        // the geometry,point type can not be converted to string, so we convert it to Object first.
+        Map<String, Object> jsonMap =
+                objectMapper.convertValue(recordRow, new TypeReference<Map<String, Object>>() {});
+
+        if (jsonMap == null) {
             return new HashMap<>();
         }
 
+        Map<String, String> resultMap = new HashMap<>();
+        for (Map.Entry<String, Object> field : jsonMap.entrySet()) {

Review Comment:
   Do we need this?



-- 
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@paimon.apache.org

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


[GitHub] [incubator-paimon] zhangjun0x01 commented on a diff in pull request #1026: [flink]add spatial type for mysql cdc action

Posted by "zhangjun0x01 (via GitHub)" <gi...@apache.org>.
zhangjun0x01 commented on code in PR #1026:
URL: https://github.com/apache/incubator-paimon/pull/1026#discussion_r1177298218


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/action/cdc/mysql/MySqlDebeziumJsonEventParser.java:
##########
@@ -200,8 +203,10 @@ public List<CdcRecord> getRecords() {
     }
 
     private Map<String, String> extractRow(JsonNode recordRow) {
-        Map<String, String> recordMap =
-                objectMapper.convertValue(recordRow, new TypeReference<Map<String, String>>() {});
+
+        // the geometry,point type can not be converted to string, so we convert it to Object first.
+        Map<String, Object> recordMap =

Review Comment:
   I update all comments .



-- 
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@paimon.apache.org

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


[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1026: [flink]add spatial type for mysql cdc action

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1026:
URL: https://github.com/apache/incubator-paimon/pull/1026#discussion_r1177273324


##########
paimon-flink/paimon-flink-common/pom.xml:
##########
@@ -81,6 +82,18 @@ under the License.
             <version>${frocksdbjni.version}</version>
         </dependency>
 
+        <dependency>
+            <groupId>com.esri.geometry</groupId>
+            <artifactId>esri-geometry-api</artifactId>
+            <version>${geometry.version}</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>com.fasterxml.jackson.core</groupId>
+                    <artifactId>jackson-core</artifactId>
+                </exclusion>
+            </exclusions>

Review Comment:
   provided?



##########
paimon-flink/paimon-flink-common/pom.xml:
##########
@@ -81,6 +82,18 @@ under the License.
             <version>${frocksdbjni.version}</version>
         </dependency>
 
+        <dependency>

Review Comment:
   Put this together with cdc dependency, and add comment for what.



##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/action/cdc/mysql/MySqlDebeziumJsonEventParser.java:
##########
@@ -200,8 +203,10 @@ public List<CdcRecord> getRecords() {
     }
 
     private Map<String, String> extractRow(JsonNode recordRow) {
-        Map<String, String> recordMap =
-                objectMapper.convertValue(recordRow, new TypeReference<Map<String, String>>() {});
+
+        // the geometry,point type can not be converted to string, so we convert it to Object first.
+        Map<String, Object> recordMap =

Review Comment:
   Can we just rename this to `jsonMap`?
   And create a new map `Map<String, String> resultMap`.



-- 
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@paimon.apache.org

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


[GitHub] [incubator-paimon] zhangjun0x01 commented on pull request #1026: [flink]add spatial type for mysql cdc action

Posted by "zhangjun0x01 (via GitHub)" <gi...@apache.org>.
zhangjun0x01 commented on PR #1026:
URL: https://github.com/apache/incubator-paimon/pull/1026#issuecomment-1525928058

   <img width="1100" alt="image" src="https://user-images.githubusercontent.com/25563794/234915212-7fada9bf-dbcf-4f4d-9497-dd0d6e16601f.png">
   


-- 
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@paimon.apache.org

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


[GitHub] [incubator-paimon] zhangjun0x01 commented on pull request #1026: [flink]add spatial type for mysql cdc action

Posted by "zhangjun0x01 (via GitHub)" <gi...@apache.org>.
zhangjun0x01 commented on PR #1026:
URL: https://github.com/apache/incubator-paimon/pull/1026#issuecomment-1522780799

   <img width="1066" alt="image" src="https://user-images.githubusercontent.com/25563794/234473567-c815722c-6d70-4fc6-8cf6-955208483ab0.png">
   


-- 
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@paimon.apache.org

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


[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1026: [flink]add spatial type for mysql cdc action

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1026:
URL: https://github.com/apache/incubator-paimon/pull/1026#discussion_r1178576281


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/action/cdc/mysql/MySqlDebeziumJsonEventParser.java:
##########
@@ -200,18 +203,27 @@ public List<CdcRecord> getRecords() {
     }
 
     private Map<String, String> extractRow(JsonNode recordRow) {
-        Map<String, String> recordMap =
-                objectMapper.convertValue(recordRow, new TypeReference<Map<String, String>>() {});
-        if (recordMap == null) {
+
+        // the geometry,point type can not be converted to string, so we convert it to Object first.
+        Map<String, Object> jsonMap =
+                objectMapper.convertValue(recordRow, new TypeReference<Map<String, Object>>() {});
+
+        if (jsonMap == null) {
             return new HashMap<>();
         }
 
+        Map<String, String> resultMap = new HashMap<>();
+        for (Map.Entry<String, Object> field : jsonMap.entrySet()) {

Review Comment:
   I know, but subsequent for loop will assign each key? Why assign 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@paimon.apache.org

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


[GitHub] [incubator-paimon] JingsongLi merged pull request #1026: [flink]add spatial type for mysql cdc action

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #1026:
URL: https://github.com/apache/incubator-paimon/pull/1026


-- 
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@paimon.apache.org

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


[GitHub] [incubator-paimon] zhangjun0x01 commented on pull request #1026: [flink]add spatial type for mysql cdc action

Posted by "zhangjun0x01 (via GitHub)" <gi...@apache.org>.
zhangjun0x01 commented on PR #1026:
URL: https://github.com/apache/incubator-paimon/pull/1026#issuecomment-1534114252

   
   <img width="1072" alt="image" src="https://user-images.githubusercontent.com/25563794/236118982-d8cd2c3f-24fc-471c-8186-d424238d0819.png">
   
   
   there is an exception in core module ,I fix it  in other pr https://github.com/apache/incubator-paimon/pull/1060
   ```
   
   Error:  Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 6.145 s <<< FAILURE! - in org.apache.paimon.operation.PartitionExpireTest
   Error:  testFilterCommittedAfterExpiring  Time elapsed: 5.714 s  <<< ERROR!
   java.time.format.DateTimeParseException: Text '20230499' could not be parsed: Invalid value for DayOfMonth (valid values 1 - 28/31): 99
   	at java.time.format.DateTimeFormatter.createError(DateTimeFormatter.java:1920)
   	at java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1855)
   	at java.time.LocalDate.parse(LocalDate.java:400)
   	at org.apache.paimon.partition.PartitionTimeExtractor.toLocalDateTime(PartitionTimeExtractor.java:112)
   	at org.apache.paimon.partition.PartitionTimeExtractor.extract(PartitionTimeExtractor.java:97)
   	at org.apache.paimon.operation.PartitionExpire.doExpire(PartitionExpire.java:95)
   	at org.apache.paimon.operation.PartitionExpire.expire(PartitionExpire.java:85)
   	at org.apache.paimon.operation.PartitionExpire.expire(PartitionExpire.java:74)
   	at org.apache.paimon.table.sink.TableCommitImpl.expire(TableCommitImpl.java:145)
   	at org.apache.paimon.table.sink.TableCommitImpl.commit(TableCommitImpl.java:105)
   	at org.apache.paimon.operation.PartitionExpireTest.testFilterCommittedAfterExpiring(PartitionExpireTest.java:189)
   	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
   	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.jav
   
   
   ```
   


-- 
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@paimon.apache.org

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


[GitHub] [incubator-paimon] zhangjun0x01 commented on a diff in pull request #1026: [flink]add spatial type for mysql cdc action

Posted by "zhangjun0x01 (via GitHub)" <gi...@apache.org>.
zhangjun0x01 commented on code in PR #1026:
URL: https://github.com/apache/incubator-paimon/pull/1026#discussion_r1177564697


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/action/cdc/mysql/MySqlDebeziumJsonEventParser.java:
##########
@@ -200,18 +203,27 @@ public List<CdcRecord> getRecords() {
     }
 
     private Map<String, String> extractRow(JsonNode recordRow) {
-        Map<String, String> recordMap =
-                objectMapper.convertValue(recordRow, new TypeReference<Map<String, String>>() {});
-        if (recordMap == null) {
+
+        // the geometry,point type can not be converted to string, so we convert it to Object first.
+        Map<String, Object> jsonMap =
+                objectMapper.convertValue(recordRow, new TypeReference<Map<String, Object>>() {});
+
+        if (jsonMap == null) {
             return new HashMap<>();
         }
 
+        Map<String, String> resultMap = new HashMap<>();
+        for (Map.Entry<String, Object> field : jsonMap.entrySet()) {

Review Comment:
   This is for the convenience of handling the subsequent logic. The `oldValue` required  in the for loop is a string type, and the return value is `Map<String, String>`. Therefore, we need to convert `Map<String, Object>` to `Map<String, String>`



-- 
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@paimon.apache.org

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