rdblue commented on code in PR #8147:
URL: https://github.com/apache/iceberg/pull/8147#discussion_r1296187471
##########
core/src/main/java/org/apache/iceberg/view/ViewMetadata.java:
##########
@@ -91,41 +109,215 @@ default ViewMetadata checkAndNormalize() {
"Unsupported format version: %s",
formatVersion());
- Preconditions.checkArgument(versions().size() > 0, "Invalid view versions:
empty");
- Preconditions.checkArgument(history().size() > 0, "Invalid view history:
empty");
- Preconditions.checkArgument(schemas().size() > 0, "Invalid schemas:
empty");
+ return this;
+ }
+
+ static Builder builder() {
+ return new Builder();
+ }
- Preconditions.checkArgument(
- versionsById().containsKey(currentVersionId()),
- "Cannot find current version %s in view versions: %s",
- currentVersionId(),
- versionsById().keySet());
+ static Builder buildFrom(ViewMetadata base) {
+ return new Builder(base);
+ }
- Preconditions.checkArgument(
- schemasById().containsKey(currentSchemaId()),
- "Cannot find current schema with id %s in schemas: %s",
- currentSchemaId(),
- schemasById().keySet());
+ class Builder {
+ private int formatVersion = DEFAULT_VIEW_FORMAT_VERSION;
+ private String location;
+ private List<Schema> schemas = Lists.newArrayList();
+ private int currentVersionId;
+ private List<ViewVersion> versions = Lists.newArrayList();
+ private List<ViewHistoryEntry> history = Lists.newArrayList();
+ private Map<String, String> properties = Maps.newHashMap();
+ private List<MetadataUpdate> changes = Lists.newArrayList();
- int versionHistorySizeToKeep =
- PropertyUtil.propertyAsInt(
- properties(),
- ViewProperties.VERSION_HISTORY_SIZE,
- ViewProperties.VERSION_HISTORY_SIZE_DEFAULT);
-
- if (versionHistorySizeToKeep <= 0) {
- LOG.warn(
- "{} must be positive but was {}",
- ViewProperties.VERSION_HISTORY_SIZE,
- versionHistorySizeToKeep);
- } else if (versions().size() > versionHistorySizeToKeep) {
- List<ViewVersion> versions =
- versions().subList(versions().size() - versionHistorySizeToKeep,
versions().size());
- List<ViewHistoryEntry> history =
- history().subList(history().size() - versionHistorySizeToKeep,
history().size());
- return
ImmutableViewMetadata.builder().from(this).versions(versions).history(history).build();
+ private Builder() {}
+
+ private Builder(ViewMetadata base) {
+ this.formatVersion = base.formatVersion();
+ this.location = base.location();
+ this.schemas = Lists.newArrayList(base.schemas());
+ this.currentVersionId = base.currentVersionId();
+ this.versions = Lists.newArrayList(base.versions());
+ this.history = Lists.newArrayList(base.history());
+ this.properties = Maps.newHashMap(base.properties());
+ this.changes = Lists.newArrayList(base.changes());
}
- return this;
+ public Builder upgradeFormatVersion(int newFormatVersion) {
+ Preconditions.checkArgument(
+ newFormatVersion >= this.formatVersion,
+ "Cannot downgrade v%s view to v%s",
+ formatVersion,
+ newFormatVersion);
+
+ if (this.formatVersion == newFormatVersion) {
+ return this;
+ }
+
+ this.formatVersion = newFormatVersion;
+ this.changes.add(new
MetadataUpdate.UpgradeFormatVersion(newFormatVersion));
+ return this;
+ }
+
+ public Builder setLocation(String newLocation) {
+ Preconditions.checkArgument(null != newLocation, "Invalid location:
null");
+ if (null != this.location && this.location.equals(newLocation)) {
+ return this;
+ }
+
+ this.location = newLocation;
+ this.changes.add(new MetadataUpdate.SetLocation(newLocation));
+ return this;
+ }
+
+ public Builder setCurrentVersionId(int newVersionId) {
+ if (this.currentVersionId == newVersionId) {
+ return this;
+ }
+
+ this.currentVersionId = newVersionId;
+ this.changes.add(new MetadataUpdate.SetCurrentViewVersion(newVersionId));
Review Comment:
I think this needs to handle version ID `-1` and track `lastAddedVersionId`
just like the `TableMetadata` builder.
Because the view version is assigned on the client, two clients could
attempt to create the same view version. We have the same situation for schemas
in table metadata. In the table metadata builder, we call
`reuseOrCreateNewSchemaId` to reassign the schema ID when a schema is added.
Then, the `setCurrentSchema` method detects whether the ID was the last added
ID and sends `-1` to signal that case. That way the server side can handle a
retry, assign a different schema ID, and still succeed.
We need to do something similar here. I think that means adding another
method, `setCurrentVersion(ViewVersion)` like this:
```java
public Builder setCurrentVersion(ViewVersion version) {
Schema schema = schemasById.get(version.schemaId());
int newSchemaId = addSchemaInternal(schema);
return setCurrentVersionId(addVersionInternal(version), newSchemaId);
}
```
You can check out how this is done in the `TableMetadata` builder.
We'll also need to handle `LAST_ADDED=-1` in this method and support `-1`
for schemas as well.
--
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]
