rdblue commented on a change in pull request #2258:
URL: https://github.com/apache/iceberg/pull/2258#discussion_r588809379
##########
File path: core/src/main/java/org/apache/iceberg/BaseFile.java
##########
@@ -434,7 +433,7 @@ public Integer sortOrderId() {
if (map != null) {
Map<K, V> copy = Maps.newHashMapWithExpectedSize(map.size());
copy.putAll(map);
- return Collections.unmodifiableMap(copy);
Review comment:
@yyanyy is right. This is a method for defensive copies. I think it can
be fine to do this, but we want to provide the same assurance that the data
won't be modified. We can do that by moving the unmodifiable map creation to
other places, but that is a big change and will result in a lot more object
creation.
Another option is to use an alternative wrapper class that can be cleanly
serialized. Elsewhere in this class we use `SerializableByteBufferMap`. Does
that work with Kryo? If so, can we use the same pattern here?
That uses a proxy object. If that works, then the proxy object can be used
to add items to the map and then the actual map could be unmodifiable.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
