[GitHub] [inlong] thexiay commented on a diff in pull request #8120: [INLONG-7959][Sort] Dynamic schema evolution support delete and update columns when sink to Iceberg

["thexiay (via GitHub)" <gi...@apache.org>]

thexiay commented on code in PR #8120:
URL: https://github.com/apache/inlong/pull/8120#discussion_r1209655765


##########
inlong-sort/sort-flink/sort-flink-v1.13/sort-connectors/iceberg/src/main/java/org/apache/inlong/sort/iceberg/sink/multiple/SchemaChangeUtils.java:
##########
@@ -52,49 +54,85 @@ public class SchemaChangeUtils {
     static List<TableChange> diffSchema(Schema oldSchema, Schema newSchema) {
         List<String> oldFields = oldSchema.columns().stream().map(NestedField::name).collect(Collectors.toList());
         List<String> newFields = newSchema.columns().stream().map(NestedField::name).collect(Collectors.toList());
-        int oi = 0;
-        int ni = 0;
+        Set<String> oldFieldSet = new HashSet<>(oldFields);
+        Set<String> newFieldSet = new HashSet<>(newFields);
+
+        Set<String> intersectColSet = Sets.intersection(oldFieldSet, newFieldSet);
+        Set<String> colsToDelete = Sets.difference(oldFieldSet, newFieldSet);
+        Set<String> colsToAdd = Sets.difference(newFieldSet, oldFieldSet);
+
         List<TableChange> tableChanges = new ArrayList<>();
-        while (ni < newFields.size()) {
-            if (oi < oldFields.size() && oldFields.get(oi).equals(newFields.get(ni))) {
-                oi++;
-                ni++;
-            } else {
-                NestedField newField = newSchema.findField(newFields.get(ni));
+
+        // step0: judge whether unknown change
+        // just diff two different schema can not distinguish（add + delete) vs modify
+        // Example first [a, b, c] -> then delete c [a, b] -> add d [a, b, d], currently it is only judged as unknown
+        // change.
+        // In next version,we will judge it is [delete and add] or rename by using information extracted from DDL
+        if (!colsToDelete.isEmpty() && !colsToAdd.isEmpty()) {

Review Comment:
   whether to add a reorder column check?



-- 
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: commits-unsubscribe@inlong.apache.org

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