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 2022/05/18 15:34:32 UTC

[GitHub] [iceberg] southernriver opened a new pull request, #4809: Recover the schema by reading previous metadata files

southernriver opened a new pull request, #4809:
URL: https://github.com/apache/iceberg/pull/4809

    If snapshot was created before Iceberg added schema id to snapshot,schemaId could be null,  we could recover the schema by reading previous metadata files


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


[GitHub] [iceberg] kbendick commented on a diff in pull request #4809: Recover the schema by reading previous metadata files

Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #4809:
URL: https://github.com/apache/iceberg/pull/4809#discussion_r876580399


##########
core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java:
##########
@@ -307,7 +307,10 @@ public static Schema schemaFor(Table table, long snapshotId) {
       return schema;
     }
 
-    // TODO: recover the schema by reading previous metadata files
+    snapshot = table.snapshot(snapshot.parentId());
+    if (snapshot != null) {
+      return table.schemas().get(snapshot.schemaId());

Review Comment:
   Is it possible that this can throw?
   
   It would be great to add a test case that covers this situation.



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


[GitHub] [iceberg] singhpk234 commented on a diff in pull request #4809: Recover the schema by reading previous metadata files

Posted by GitBox <gi...@apache.org>.
singhpk234 commented on code in PR #4809:
URL: https://github.com/apache/iceberg/pull/4809#discussion_r876957920


##########
core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java:
##########
@@ -307,7 +307,10 @@ public static Schema schemaFor(Table table, long snapshotId) {
       return schema;
     }
 
-    // TODO: recover the schema by reading previous metadata files
+    snapshot = table.snapshot(snapshot.parentId());

Review Comment:
   [minor] should we rename this`parentSnapshot` ?



##########
core/src/main/java/org/apache/iceberg/util/SnapshotUtil.java:
##########
@@ -307,7 +307,10 @@ public static Schema schemaFor(Table table, long snapshotId) {
       return schema;
     }
 
-    // TODO: recover the schema by reading previous metadata files
+    snapshot = table.snapshot(snapshot.parentId());
+    if (snapshot != null) {
+      return table.schemas().get(snapshot.schemaId());

Review Comment:
   [question] can it be possible that even parentSnapshot is also lacking schemaId ? 
   
   If this is true we can now return `null` earlier we were returning `table.schema()` instead. WDYT ?



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


[GitHub] [iceberg] wypoon commented on pull request #4809: Core: Recover the schema by reading previous metadata files

Posted by GitBox <gi...@apache.org>.
wypoon commented on PR #4809:
URL: https://github.com/apache/iceberg/pull/4809#issuecomment-1131993501

   I don't think this is the way to do it. If the snapshot is written before schema ids were introduced, then most likely its ancestors also do not have schema ids, unless different versions of Iceberg are in use concurrently, and one is of a version before schema ids were introduced.
   I put up #3852 months ago to perform the recovery, and also added tests for the recovery. @kbendick can you please review?


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