Skip to content

API, Core: Move schemaID from ViewRepresentation to ViewVersion and make it required#7421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jackye1995 merged 1 commit into from
Apr 25, 2023
Merged

API, Core: Move schemaID from ViewRepresentation to ViewVersion and make it required #7421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .palantir/revapi.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -410,9 +410,30 @@ acceptedBreaks:
old: "field org.apache.iceberg.actions.RewriteDataFiles.MAX_CONCURRENT_FILE_GROUP_REWRITES_DEFAULT"
new: "field org.apache.iceberg.actions.RewriteDataFiles.MAX_CONCURRENT_FILE_GROUP_REWRITES_DEFAULT"
justification: "Update default to align with recommendation and allow more parallelism"
- code: "java.method.addedToInterface"
new: "method int org.apache.iceberg.view.ViewVersion::schemaId()"
justification: "Moving SchemaID to ViewVersion. View Spec implementation has\
\ not been voted on yet so this break is acceptable"
- code: "java.method.addedToInterface"
new: "method java.lang.String org.apache.iceberg.view.ViewVersion::operation()"
justification: "Add operation API to view version"
- code: "java.method.removed"
old: "method java.lang.Integer org.apache.iceberg.view.ImmutableSQLViewRepresentation::schemaId()"
justification: "Moving SchemaID to ViewVersion. View Spec implementation has\
\ not been voted on yet so this break is acceptable"
- code: "java.method.removed"
old: "method java.lang.Integer org.apache.iceberg.view.SQLViewRepresentation::schemaId()"
justification: "Moving SchemaID to ViewVersion. View Spec implementation has\
\ not been voted on yet so this break is acceptable"
- code: "java.method.removed"
old: "method org.apache.iceberg.view.ImmutableSQLViewRepresentation org.apache.iceberg.view.ImmutableSQLViewRepresentation::withSchemaId(java.lang.Integer)"
justification: "Moving SchemaID to ViewVersion. View Spec implementation has\
\ not been voted on yet so this break is acceptable"
- code: "java.method.removed"
old: "method org.apache.iceberg.view.ImmutableSQLViewRepresentation.Builder\
\ org.apache.iceberg.view.ImmutableSQLViewRepresentation.Builder::schemaId(java.lang.Integer)"
justification: "Moving SchemaID to ViewVersion. View Spec implementation has\
\ not been voted on yet so this break is acceptable"
org.apache.iceberg:iceberg-core:
- code: "java.class.removed"
old: "class org.apache.iceberg.actions.BaseExpireSnapshotsActionResult"
Expand Down
7 changes: 0 additions & 7 deletions api/src/main/java/org/apache/iceberg/view/SQLViewRepresentation.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,13 +45,6 @@ default String type() {
@Nullable
NamespacedefaultNamespace();

/**
* The query output schema ID at version create time, without aliases or null if no schema is
* defined
*/
@Nullable
IntegerschemaId();

/** The view field comments. */
List<String> fieldComments();

Expand Down
3 changes: 3 additions & 0 deletions api/src/main/java/org/apache/iceberg/view/ViewVersion.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,4 +69,7 @@ public interface ViewVersion {
defaultStringoperation() {
returnsummary().get("operation");
}

/** The query output schema at version create time, without aliases */
intschemaId();
}
7 changes: 0 additions & 7 deletions core/src/main/java/org/apache/iceberg/view/SQLViewRepresentationParser.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -50,10 +50,6 @@ static void toJson(SQLViewRepresentation view, JsonGenerator generator) throws I
generator.writeStringField(SQL, view.sql());
generator.writeStringField(DIALECT, view.dialect());

if (view.schemaId() != null) {
generator.writeNumberField(SCHEMA_ID, view.schemaId());
}

if (view.defaultCatalog() != null) {
generator.writeStringField(DEFAULT_CATALOG, view.defaultCatalog());
}
Expand DownExpand Up@@ -93,9 +89,6 @@ static SQLViewRepresentation fromJson(JsonNode node) {
}

IntegerschemaId = JsonUtil.getIntOrNull(SCHEMA_ID, node);
if (schemaId != null) {
builder.schemaId(schemaId);
}

List<String> namespace = JsonUtil.getStringListOrNull(DEFAULT_NAMESPACE, node);
if (namespace != null && !namespace.isEmpty()) {
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/org/apache/iceberg/view/ViewVersionParser.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -33,6 +33,7 @@ class ViewVersionParser {
privatestaticfinalStringSUMMARY = "summary";
privatestaticfinalStringOPERATION = "operation";
privatestaticfinalStringREPRESENTATIONS = "representations";
privatestaticfinalStringSCHEMA_ID = "schema-id";

privateViewVersionParser() {}

Expand All@@ -42,6 +43,7 @@ static void toJson(ViewVersion version, JsonGenerator generator) throws IOExcept

generator.writeNumberField(VERSION_ID, version.versionId());
generator.writeNumberField(TIMESTAMP_MS, version.timestampMillis());
generator.writeNumberField(SCHEMA_ID, version.schemaId());
Comment thread
amogh-jahagirdar marked this conversation as resolved.

generator.writeObjectFieldStart(SUMMARY);
generator.writeStringField(OPERATION, version.operation());
Expand DownExpand Up@@ -77,6 +79,7 @@ static ViewVersion fromJson(JsonNode node) {
node.isObject(), "Cannot parse view version from a non-object: %s", node);

intversionId = JsonUtil.getInt(VERSION_ID, node);
intschemaId = JsonUtil.getInt(SCHEMA_ID, node);
longtimestamp = JsonUtil.getLong(TIMESTAMP_MS, node);
Map<String, String> summary = JsonUtil.getStringMap(SUMMARY, node);
Preconditions.checkArgument(
Expand All@@ -93,6 +96,7 @@ static ViewVersion fromJson(JsonNode node) {
returnImmutableViewVersion.builder()
.versionId(versionId)
.timestampMillis(timestamp)
.schemaId(schemaId)
.summary(summary)
.representations(representations.build())
.build();
Expand Down
6 changes: 2 additions & 4 deletions core/src/test/java/org/apache/iceberg/view/TestSQLViewRepresentationParser.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -41,7 +41,7 @@ public void testParseSqlViewRepresentation() {
SQLViewRepresentationParser.fromJson(requiredFields));

String requiredAndOptionalFields =
"{\"type\":\"sql\", \"sql\": \"select * from foo\", \"schema-id\": 1, \"dialect\": \"spark-sql\", "
"{\"type\":\"sql\", \"sql\": \"select * from foo\", \"dialect\": \"spark-sql\", "
+ "\"default-catalog\":\"cat\", "
+ "\"default-namespace\":[\"part1\",\"part2\"], "
+ "\"field-aliases\":[\"col1\", \"col2\"], "
Expand All@@ -50,7 +50,6 @@ public void testParseSqlViewRepresentation() {
SQLViewRepresentation viewWithOptionalFields =
ImmutableSQLViewRepresentation.builder()
.sql("select * from foo")
.schemaId(1)
.dialect("spark-sql")
.defaultCatalog("cat")
.fieldAliases(ImmutableList.of("col1", "col2"))
Expand DownExpand Up@@ -91,7 +90,7 @@ public void testViewRepresentationSerialization() {
ViewRepresentationParser.toJson(viewRepresentation));

String requiredAndOptionalFields =
"{\"type\":\"sql\",\"sql\":\"select * from foo\",\"dialect\":\"spark-sql\",\"schema-id\":1,"
"{\"type\":\"sql\",\"sql\":\"select * from foo\",\"dialect\":\"spark-sql\","
+ "\"default-catalog\":\"cat\","
+ "\"default-namespace\":[\"part1\",\"part2\"],"
+ "\"field-aliases\":[\"col1\",\"col2\"],"
Expand All@@ -100,7 +99,6 @@ public void testViewRepresentationSerialization() {
SQLViewRepresentation viewWithOptionalFields =
ImmutableSQLViewRepresentation.builder()
.sql("select * from foo")
.schemaId(1)
.dialect("spark-sql")
.defaultCatalog("cat")
.fieldAliases(ImmutableList.of("col1", "col2"))
Expand Down
8 changes: 5 additions & 3 deletions core/src/test/java/org/apache/iceberg/view/TestViewVersionParser.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -45,6 +45,7 @@ public void testParseViewVersion() {
.timestampMillis(12345)
.addRepresentations(firstRepresentation, secondRepresentation)
.summary(ImmutableMap.of("operation", "create", "user", "some-user"))
.schemaId(1)
.build();

StringserializedRepresentations =
Expand All@@ -53,7 +54,7 @@ public void testParseViewVersion() {

StringserializedViewVersion =
String.format(
"{\"version-id\":1, \"timestamp-ms\":12345, \"summary\":{\"operation\":\"create\", \"user\":\"some-user\"}, \"representations\":%s}",
"{\"version-id\":1, \"timestamp-ms\":12345, \"schema-id\":1, \"summary\":{\"operation\":\"create\", \"user\":\"some-user\"}, \"representations\":%s}",
serializedRepresentations);

Assert.assertEquals(
Expand DownExpand Up@@ -81,6 +82,7 @@ public void testSerializeViewVersion() {
.timestampMillis(12345)
.addRepresentations(firstRepresentation, secondRepresentation)
.summary(ImmutableMap.of("operation", "create", "user", "some-user"))
.schemaId(1)
.build();

StringexpectedRepresentations =
Expand All@@ -89,7 +91,7 @@ public void testSerializeViewVersion() {

StringexpectedViewVersion =
String.format(
"{\"version-id\":1,\"timestamp-ms\":12345,\"summary\":{\"operation\":\"create\",\"user\":\"some-user\"},\"representations\":%s}",
"{\"version-id\":1,\"timestamp-ms\":12345,\"schema-id\":1,\"summary\":{\"operation\":\"create\",\"user\":\"some-user\"},\"representations\":%s}",
expectedRepresentations);

Assert.assertEquals(
Expand All@@ -106,7 +108,7 @@ public void testFailParsingMissingOperation() {

StringviewVersionMissingOperation =
String.format(
"{\"version-id\":1,\"timestamp-ms\":12345,\"summary\":{\"some-other-field\":\"some-other-value\"},\"representations\":%s}",
"{\"version-id\":1,\"timestamp-ms\":12345,\"summary\":{\"some-other-field\":\"some-other-value\"},\"representations\":%s,\"schema-id\":1}",
serializedRepresentations);

Assertions.assertThatThrownBy(() -> ViewVersionParser.fromJson(viewVersionMissingOperation))
Expand Down