RussellSpitzer commented on code in PR #10020: URL: https://github.com/apache/iceberg/pull/10020#discussion_r1633719086
########## api/src/main/java/org/apache/iceberg/Schema.java: ########## @@ -507,4 +538,49 @@ public String toString() { .map(this::identifierFieldToString) .collect(Collectors.toList()))); } + + /** + * All ids of metadata fields are reassigned. + * + * @return map of original to reassigned field ids of metadata fields + */ + public Map<Integer, Integer> idsToReassigned() { + return idsToReassigned != null ? idsToReassigned : Maps.newHashMap(); + } + + /** + * All ids of metadata fields are reassigned. + * + * @return map of reassigned to original field ids of metadata fields + */ + public Map<Integer, Integer> idsToOriginal() { + return idsToOriginal != null ? idsToOriginal : Maps.newHashMap(); + } + + private List<NestedField> reassignMetadataFieldIds( + List<NestedField> columns, Set<Integer> metadataFieldIds) { + Set<Integer> usedIds = + Sets.newHashSet( + Sets.difference(TypeUtil.indexById(StructType.of(columns)).keySet(), metadataFieldIds)); Review Comment: This is unintentionally (i think) making a mutable hash set, I think you want to do: Sets.difference(....).immutableCopy(); ```java TypeUtil.indexById(StructType.of(columns)).keySet() ``` Also seems like it may be an expensive way to just a set of all the Id's. Is this required because of the nested schema? If so then I think we are ok 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org