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

Reply via email to