twuebi commented on code in PR #575:
URL: https://github.com/apache/iceberg-go/pull/575#discussion_r2371894614
##########
table/metadata.go:
##########
@@ -1313,6 +1366,14 @@ func (c *commonMetadata) validate() error {
c.constructRefs()
+ if err := c.checkMainRefMatchesCurrentSnapshot(); err != nil {
+ return err
+ }
Review Comment:
This is called from the TableMetadata constructor of java:
```java
private static Map<String, SnapshotRef> validateRefs(
Long currentSnapshotId,
Map<String, SnapshotRef> inputRefs,
Map<Long, Snapshot> snapshotsById) {
for (SnapshotRef ref : inputRefs.values()) {
Preconditions.checkArgument(
snapshotsById.containsKey(ref.snapshotId()),
"Snapshot for reference %s does not exist in the existing
snapshots list",
ref);
}
SnapshotRef main = inputRefs.get(SnapshotRef.MAIN_BRANCH);
if (currentSnapshotId != -1) {
Preconditions.checkArgument(
main == null || currentSnapshotId == main.snapshotId(),
"Current snapshot ID does not match main branch (%s != %s)",
currentSnapshotId,
main != null ? main.snapshotId() : null);
} else {
Preconditions.checkArgument(
main == null, "Current snapshot is not set, but main branch
exists: %s", main);
}
return inputRefs;
}
```
Then there's also this check at the end of the constructor:
```java
private void validateCurrentSnapshot() {
Preconditions.checkArgument(
currentSnapshotId < 0 ||
snapshotsById.containsKey(currentSnapshotId),
"Invalid table metadata: Cannot find current version");
}
```
Parsing a TableMetadata with a main branch but without current-snapshot-id:
```json
{
...
"snapshots": [
{
"snapshot-id": 3051729675574597004,
"timestamp-ms": 1515100955770,
"sequence-number": 0,
"summary": {
"operation": "append"
},
"manifest-list": "s3://a/b/1.avro"
},
{
"snapshot-id": 3055729675574597004,
"parent-snapshot-id": 3051729675574597004,
"timestamp-ms": 1555100955770,
"sequence-number": 1,
"summary": {
"operation": "append"
},
"manifest-list": "s3://a/b/2.avro",
"schema-id": 1
}
],
"refs": {"main": {"snapshot-id": 3051729675574597004, "type": "branch"}}
}
```
leads to this:
```
Current snapshot is not set, but main branch exists:
SnapshotRef{snapshotId=3051729675574597004, type=BRANCH,
minSnapshotsToKeep=null, maxSnapshotAgeMs=null, maxRefAgeMs=null}
java.lang.IllegalArgumentException: Current snapshot is not set, but main
branch exists: SnapshotRef{snapshotId=3051729675574597004, type=BRANCH,
minSnapshotsToKeep=null, maxSnapshotAgeMs=null, maxRefAgeMs=null}
at
org.apache.iceberg.relocated.com.google.common.base.Preconditions.checkArgument(Preconditions.java:217)
at org.apache.iceberg.TableMetadata.validateRefs(TableMetadata.java:870)
at org.apache.iceberg.TableMetadata.<init>(TableMetadata.java:350)
at
org.apache.iceberg.TableMetadataParser.fromJson(TableMetadataParser.java:583)
at
org.apache.iceberg.TableMetadataParser.lambda$fromJson$0(TableMetadataParser.java:327)
at org.apache.iceberg.util.JsonUtil.parse(JsonUtil.java:104)
at
org.apache.iceberg.TableMetadataParser.fromJson(TableMetadataParser.java:327)
at
org.apache.iceberg.TableMetadataParser.fromJson(TableMetadataParser.java:316)
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
