This is an automated email from the ASF dual-hosted git repository.
etudenhoefner pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/master by this push:
new 4c1188a35b Core: Don't allow setting schema id / derive current schema
id from ViewVersion (#8210)
4c1188a35b is described below
commit 4c1188a35b601281d7418343f93d7204d7d23dda
Author: Eduard Tudenhoefner <[email protected]>
AuthorDate: Tue Aug 15 11:39:03 2023 +0200
Core: Don't allow setting schema id / derive current schema id from
ViewVersion (#8210)
---
.../java/org/apache/iceberg/view/ViewMetadata.java | 4 +-
.../apache/iceberg/view/ViewMetadataParser.java | 4 --
.../org/apache/iceberg/view/TestViewMetadata.java | 26 +-------
.../iceberg/view/TestViewMetadataParser.java | 10 ---
.../view/ViewMetadataInvalidCurrentSchema.json | 3 +-
.../view/ViewMetadataMissingCurrentSchema.json | 75 ----------------------
format/view-spec.md | 3 -
7 files changed, 7 insertions(+), 118 deletions(-)
diff --git a/core/src/main/java/org/apache/iceberg/view/ViewMetadata.java
b/core/src/main/java/org/apache/iceberg/view/ViewMetadata.java
index cfd91b02f2..00af4cf3a0 100644
--- a/core/src/main/java/org/apache/iceberg/view/ViewMetadata.java
+++ b/core/src/main/java/org/apache/iceberg/view/ViewMetadata.java
@@ -38,7 +38,9 @@ public interface ViewMetadata extends Serializable {
String location();
- Integer currentSchemaId();
+ default Integer currentSchemaId() {
+ return currentVersion().schemaId();
+ }
List<Schema> schemas();
diff --git a/core/src/main/java/org/apache/iceberg/view/ViewMetadataParser.java
b/core/src/main/java/org/apache/iceberg/view/ViewMetadataParser.java
index 57c393c447..f1fca511b1 100644
--- a/core/src/main/java/org/apache/iceberg/view/ViewMetadataParser.java
+++ b/core/src/main/java/org/apache/iceberg/view/ViewMetadataParser.java
@@ -45,7 +45,6 @@ public class ViewMetadataParser {
static final String VERSION_LOG = "version-log";
static final String PROPERTIES = "properties";
static final String SCHEMAS = "schemas";
- static final String CURRENT_SCHEMA_ID = "current-schema-id";
private ViewMetadataParser() {}
@@ -65,7 +64,6 @@ public class ViewMetadataParser {
gen.writeNumberField(FORMAT_VERSION, metadata.formatVersion());
gen.writeStringField(LOCATION, metadata.location());
JsonUtil.writeStringMap(PROPERTIES, metadata.properties(), gen);
- gen.writeNumberField(CURRENT_SCHEMA_ID, metadata.currentSchemaId());
gen.writeArrayFieldStart(SCHEMAS);
for (Schema schema : metadata.schemas()) {
@@ -103,7 +101,6 @@ public class ViewMetadataParser {
String location = JsonUtil.getString(LOCATION, json);
Map<String, String> properties = JsonUtil.getStringMap(PROPERTIES, json);
- int currentSchemaId = JsonUtil.getInt(CURRENT_SCHEMA_ID, json);
JsonNode schemasNode = JsonUtil.get(SCHEMAS, json);
Preconditions.checkArgument(
@@ -138,7 +135,6 @@ public class ViewMetadataParser {
.properties(properties)
.versions(versions)
.schemas(schemas)
- .currentSchemaId(currentSchemaId)
.history(historyEntries)
.formatVersion(formatVersion)
.build();
diff --git a/core/src/test/java/org/apache/iceberg/view/TestViewMetadata.java
b/core/src/test/java/org/apache/iceberg/view/TestViewMetadata.java
index dc881537b8..5d4b3bd0f3 100644
--- a/core/src/test/java/org/apache/iceberg/view/TestViewMetadata.java
+++ b/core/src/test/java/org/apache/iceberg/view/TestViewMetadata.java
@@ -34,27 +34,16 @@ public class TestViewMetadata {
assertThatThrownBy(() -> ImmutableViewMetadata.builder().build())
.isInstanceOf(IllegalStateException.class)
.hasMessage(
- "Cannot build ViewMetadata, some of required attributes are not
set [formatVersion, location, currentSchemaId, currentVersionId]");
+ "Cannot build ViewMetadata, some of required attributes are not
set [formatVersion, location, currentVersionId]");
assertThatThrownBy(() ->
ImmutableViewMetadata.builder().formatVersion(1).build())
.isInstanceOf(IllegalStateException.class)
.hasMessage(
- "Cannot build ViewMetadata, some of required attributes are not
set [location, currentSchemaId, currentVersionId]");
+ "Cannot build ViewMetadata, some of required attributes are not
set [location, currentVersionId]");
assertThatThrownBy(
() ->
ImmutableViewMetadata.builder().formatVersion(1).location("location").build())
.isInstanceOf(IllegalStateException.class)
- .hasMessage(
- "Cannot build ViewMetadata, some of required attributes are not
set [currentSchemaId, currentVersionId]");
-
- assertThatThrownBy(
- () ->
- ImmutableViewMetadata.builder()
- .formatVersion(1)
- .location("location")
- .currentSchemaId(1)
- .build())
- .isInstanceOf(IllegalStateException.class)
.hasMessage(
"Cannot build ViewMetadata, some of required attributes are not
set [currentVersionId]");
@@ -63,7 +52,6 @@ public class TestViewMetadata {
ImmutableViewMetadata.builder()
.formatVersion(1)
.location("location")
- .currentSchemaId(1)
.currentVersionId(1)
.build())
.isInstanceOf(IllegalArgumentException.class)
@@ -77,7 +65,6 @@ public class TestViewMetadata {
ImmutableViewMetadata.builder()
.formatVersion(23)
.location("location")
- .currentSchemaId(1)
.currentVersionId(1)
.build())
.isInstanceOf(IllegalArgumentException.class)
@@ -91,7 +78,6 @@ public class TestViewMetadata {
ImmutableViewMetadata.builder()
.formatVersion(1)
.location("location")
- .currentSchemaId(1)
.currentVersionId(1)
.build())
.isInstanceOf(IllegalArgumentException.class)
@@ -105,7 +91,6 @@ public class TestViewMetadata {
ImmutableViewMetadata.builder()
.formatVersion(1)
.location("location")
- .currentSchemaId(1)
.currentVersionId(1)
.addVersions(
ImmutableViewVersion.builder()
@@ -132,7 +117,6 @@ public class TestViewMetadata {
ImmutableViewMetadata.builder()
.formatVersion(1)
.location("location")
- .currentSchemaId(1)
.currentVersionId(23)
.addVersions(
ImmutableViewVersion.builder()
@@ -161,11 +145,10 @@ public class TestViewMetadata {
ImmutableViewMetadata.builder()
.formatVersion(1)
.location("location")
- .currentSchemaId(23)
.currentVersionId(1)
.addVersions(
ImmutableViewVersion.builder()
- .schemaId(1)
+ .schemaId(23)
.versionId(1)
.defaultNamespace(Namespace.of("ns"))
.timestampMillis(23L)
@@ -191,7 +174,6 @@ public class TestViewMetadata {
.properties(ImmutableMap.of(ViewProperties.VERSION_HISTORY_SIZE,
"0"))
.formatVersion(1)
.location("location")
- .currentSchemaId(1)
.currentVersionId(3)
.addVersions(
ImmutableViewVersion.builder()
@@ -235,7 +217,6 @@ public class TestViewMetadata {
ImmutableViewMetadata.builder()
.formatVersion(1)
.location("location")
- .currentSchemaId(1)
.currentVersionId(2)
.addVersions(
ImmutableViewVersion.builder()
@@ -267,7 +248,6 @@ public class TestViewMetadata {
.properties(ImmutableMap.of(ViewProperties.VERSION_HISTORY_SIZE,
"1"))
.formatVersion(1)
.location("location")
- .currentSchemaId(1)
.currentVersionId(3)
.addVersions(
ImmutableViewVersion.builder()
diff --git
a/core/src/test/java/org/apache/iceberg/view/TestViewMetadataParser.java
b/core/src/test/java/org/apache/iceberg/view/TestViewMetadataParser.java
index 0fed278b3f..6a046d980f 100644
--- a/core/src/test/java/org/apache/iceberg/view/TestViewMetadataParser.java
+++ b/core/src/test/java/org/apache/iceberg/view/TestViewMetadataParser.java
@@ -96,7 +96,6 @@ public class TestViewMetadataParser {
String json =
readViewMetadataInputFile("org/apache/iceberg/view/ValidViewMetadata.json");
ViewMetadata expectedViewMetadata =
ImmutableViewMetadata.builder()
- .currentSchemaId(1)
.schemas(ImmutableList.of(TEST_SCHEMA))
.versions(ImmutableList.of(version1, version2))
.history(ImmutableList.of(historyEntry1, historyEntry2))
@@ -144,15 +143,6 @@ public class TestViewMetadataParser {
.hasMessage("Cannot parse missing string: location");
}
- @Test
- public void failReadingViewMetadataMissingCurrentSchema() throws Exception {
- String json =
-
readViewMetadataInputFile("org/apache/iceberg/view/ViewMetadataMissingCurrentSchema.json");
- assertThatThrownBy(() -> ViewMetadataParser.fromJson(json))
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessage("Cannot parse missing int: current-schema-id");
- }
-
@Test
public void failReadingViewMetadataInvalidSchemaId() throws Exception {
String json =
diff --git
a/core/src/test/resources/org/apache/iceberg/view/ViewMetadataInvalidCurrentSchema.json
b/core/src/test/resources/org/apache/iceberg/view/ViewMetadataInvalidCurrentSchema.json
index b90befe515..b63c6a6285 100644
---
a/core/src/test/resources/org/apache/iceberg/view/ViewMetadataInvalidCurrentSchema.json
+++
b/core/src/test/resources/org/apache/iceberg/view/ViewMetadataInvalidCurrentSchema.json
@@ -2,7 +2,6 @@
"format-version": 1,
"location": "s3://bucket/test/location",
"properties": {"some-key": "some-value"},
- "current-schema-id": 1234,
"schemas": [
{
"type": "struct",
@@ -51,7 +50,7 @@
"version-id": 2,
"timestamp-ms": 5555,
"summary": {"operation": "replace"},
- "schema-id": 1,
+ "schema-id": 1234,
"default-catalog": "some-catalog",
"default-namespace": [],
"representations": [
diff --git
a/core/src/test/resources/org/apache/iceberg/view/ViewMetadataMissingCurrentSchema.json
b/core/src/test/resources/org/apache/iceberg/view/ViewMetadataMissingCurrentSchema.json
deleted file mode 100644
index f2dc76723d..0000000000
---
a/core/src/test/resources/org/apache/iceberg/view/ViewMetadataMissingCurrentSchema.json
+++ /dev/null
@@ -1,75 +0,0 @@
-{
- "format-version": 1,
- "location": "s3://bucket/test/location",
- "properties": {"some-key": "some-value"},
- "schemas": [
- {
- "type": "struct",
- "schema-id": 1,
- "fields": [
- {
- "id": 1,
- "name": "x",
- "required": true,
- "type": "long"
- },
- {
- "id": 2,
- "name": "y",
- "required": true,
- "type": "long",
- "doc": "comment"
- },
- {
- "id": 3,
- "name": "z",
- "required": true,
- "type": "long"
- }
- ]
- }
- ],
- "current-version-id": 2,
- "versions": [
- {
- "version-id": 1,
- "timestamp-ms": 4353,
- "summary": {"operation":"create"},
- "schema-id": 1,
- "default-catalog": "some-catalog",
- "default-namespace": [],
- "representations": [
- {
- "type": "sql",
- "sql": "select 'foo' foo",
- "dialect": "spark-sql"
- }
- ]
- },
- {
- "version-id": 2,
- "timestamp-ms": 5555,
- "summary": {"operation": "replace"},
- "schema-id": 1,
- "default-catalog": "some-catalog",
- "default-namespace": [],
- "representations": [
- {
- "type": "sql",
- "sql": "select 1 id, 'abc' data",
- "dialect": "spark-sql"
- }
- ]
- }
- ],
- "version-log": [
- {
- "timestamp-ms": 4353,
- "version-id": 1
- },
- {
- "timestamp-ms": 5555,
- "version-id": 2
- }
- ]
-}
\ No newline at end of file
diff --git a/format/view-spec.md b/format/view-spec.md
index fe23b4efb8..d7e0b7b7a6 100644
--- a/format/view-spec.md
+++ b/format/view-spec.md
@@ -60,7 +60,6 @@ The view version metadata file has the following fields:
|-------------|----------------------|-------------|
| _required_ | `format-version` | An integer version number for the view
format; must be 1 |
| _required_ | `location` | The view's base location; used to
create metadata file locations |
-| _required_ | `current-schema-id` | ID of the current schema of the view,
if known |
| _required_ | `schemas` | A list of known schemas |
| _required_ | `current-version-id` | ID of the current version of the view
(`version-id`) |
| _required_ | `versions` | A list of known [versions](#versions)
of the view [1] |
@@ -216,7 +215,6 @@
s3://bucket/warehouse/default.db/event_agg/metadata/00001-(uuid).metadata.json
"dialect" : "spark"
} ]
} ],
- "current-schema-id": 1,
"schemas": [ {
"schema-id": 1,
"type" : "struct",
@@ -300,7 +298,6 @@
s3://bucket/warehouse/default.db/event_agg/metadata/00002-(uuid).metadata.json
"dialect" : "spark"
} ]
} ],
- "current-schema-id": 1,
"schemas": [ {
"schema-id": 1,
"type" : "struct",
