[GitHub] [iceberg] findepi commented on a diff in pull request #4741: Add implementation for statistics information in table snapshot

[GitBox <gi...@apache.org>]

findepi commented on code in PR #4741:
URL: https://github.com/apache/iceberg/pull/4741#discussion_r966768842


##########
core/src/main/java/org/apache/iceberg/GenericBlobMetadata.java:
##########
@@ -26,7 +27,17 @@
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
 
-public class GenericBlobMetadata implements BlobMetadata {
+public class GenericBlobMetadata implements BlobMetadata, Serializable {
+
+  public static BlobMetadata from(org.apache.iceberg.puffin.BlobMetadata puffinMetadata) {

Review Comment:
   Puffin BlobMetadata contains all the blob metadata information from the Puffin footer.
   In particular, it contains more fields, like offset and length, which are not carried over to the table metadata.
   
   We can make Puffin `BlobMetadata` implement the `BlobMetadata`, but
   - we don't want to serialize (as in `Serializable`) the Puffin BlobMetadata, as we're not interested in sending over those additional fields (I know these are small today, and _maybe_ are never big in the future)
   - there is a challenge with equality semantics. `GenericBlobMetadata` implements equals, but with more complicated class hierarchy it wouldn't be useful anymore.
   
   Let me know what you think



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org