leaves12138 commented on code in PR #7933:
URL: https://github.com/apache/paimon/pull/7933#discussion_r3323154956


##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/dataevolution/MergeIntoUpdateChecker.java:
##########
@@ -159,4 +165,23 @@ private void checkUpdatedColumns() {
             }
         }
     }
+
+    private static Collection<String> getIndexedFieldNames(GlobalIndexMeta 
meta, RowType rowType) {
+        int fieldId = meta.indexFieldId();
+        if (fieldId == MULTI_COLUMN_INDEX_FIELD_ID) {
+            List<String> names = new ArrayList<>();
+            for (int id : meta.extraFieldIds()) {
+                names.add(rowType.getField(id).name());
+            }
+            return names;
+        }
+        List<String> names = new ArrayList<>();
+        names.add(rowType.getField(fieldId).name());
+        if (meta.extraFieldIds() != null) {
+            for (int id : meta.extraFieldIds()) {
+                names.add(rowType.getField(id).name());
+            }
+        }
+        return names;
+    }
 }

Review Comment:
   **Code duplication — `getIndexedFieldNames` is copy-pasted 3 times**
   
   This same helper logic exists in:
   1. Here — `MergeIntoUpdateChecker.java` (Flink, Java)
   2. `MergeIntoPaimonDataEvolutionTable.scala` (Spark common, Scala)
   3. `MergeIntoPaimonDataEvolutionTable.scala` (Spark 4.0, Scala)
   
   Consider extracting it as a static method in `GlobalIndexMeta` or 
`GlobalIndexBuilderUtils` so all three call sites can reuse a single 
implementation. This also reduces the risk of future inconsistencies when the 
logic evolves.



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