You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2020/04/22 22:38:20 UTC

[GitHub] [incubator-iceberg] rdblue opened a new pull request #952: Merge ManifestEntry and DataFile classes

rdblue opened a new pull request #952:
URL: https://github.com/apache/incubator-iceberg/pull/952


   This updates `DataFile` to contain metadata from `ManifestEntry` because the separation no longer makes sense. V1 metadata files still use `ManifestEntry`, but v2 will not.
   
   `GenericManifestEntry` is split into two parts. First, a wrapper class `ManifestEntryWrapper` that is used by `ManifestWriter` to replace fields like status when writing a `DataFile` because the `DataFile` status is not set by the caller. Second, a class that makes a `DataFile` appear like a `ManifestEntry`, called `DataFile.AsManifestEntry`. The `GenericManifestEntry` class now extends `AsManifestEntry` so that reads that used it don't need to be updated.


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-iceberg] rdblue commented on pull request #952: Merge ManifestEntry and DataFile classes

Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #952:
URL: https://github.com/apache/incubator-iceberg/pull/952#issuecomment-625371050


   I'm closing this because we plan to keep the separate structs.


----------------------------------------------------------------
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:
users@infra.apache.org



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


[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #952: Merge ManifestEntry and DataFile classes

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #952:
URL: https://github.com/apache/incubator-iceberg/pull/952#discussion_r413926234



##########
File path: core/src/main/java/org/apache/iceberg/ManifestEntry.java
##########
@@ -54,7 +57,12 @@ static Schema getSchema(StructType partitionType) {
   }
 
   static Schema wrapFileSchema(StructType fileType) {
-    return new Schema(STATUS, SNAPSHOT_ID, SEQUENCE_NUMBER, required(DATA_FILE_ID, "data_file", fileType));
+    // remove ManifestEntry fields from the file type when wrapping to avoid duplication

Review comment:
       I should note that this PR doesn't actually add the manifest entry fields to `DataFile`, it just sets up that change.




----------------------------------------------------------------
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:
users@infra.apache.org



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