You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ja...@apache.org on 2023/04/25 04:38:59 UTC
[iceberg] branch master updated: Views: Fix SQL view representation field name (#7417)
This is an automated email from the ASF dual-hosted git repository.
jackye 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 973676a1bd Views: Fix SQL view representation field name (#7417)
973676a1bd is described below
commit 973676a1bd0c6bdfaf858da8f22c7c503c985de4
Author: Ryan Blue <bl...@apache.org>
AuthorDate: Mon Apr 24 21:38:52 2023 -0700
Views: Fix SQL view representation field name (#7417)
---
.../java/org/apache/iceberg/view/ViewVersion.java | 5 ++++-
.../iceberg/view/SQLViewRepresentationParser.java | 6 ++---
.../iceberg/view/ViewRepresentationParser.java | 7 +++---
.../org/apache/iceberg/view/ViewVersionParser.java | 26 +++++-----------------
.../iceberg/view/TestViewRepresentationParser.java | 4 ++--
.../apache/iceberg/view/TestViewVersionParser.java | 8 +++----
6 files changed, 20 insertions(+), 36 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/view/ViewVersion.java b/api/src/main/java/org/apache/iceberg/view/ViewVersion.java
index 3e7dfa0a5f..6d6c0cf283 100644
--- a/api/src/main/java/org/apache/iceberg/view/ViewVersion.java
+++ b/api/src/main/java/org/apache/iceberg/view/ViewVersion.java
@@ -65,5 +65,8 @@ public interface ViewVersion {
*
* @return the string operation which produced the view version
*/
- String operation();
+ @Value.Derived
+ default String operation() {
+ return summary().get("operation");
+ }
}
diff --git a/core/src/main/java/org/apache/iceberg/view/SQLViewRepresentationParser.java b/core/src/main/java/org/apache/iceberg/view/SQLViewRepresentationParser.java
index 57cd5c65c6..9100822196 100644
--- a/core/src/main/java/org/apache/iceberg/view/SQLViewRepresentationParser.java
+++ b/core/src/main/java/org/apache/iceberg/view/SQLViewRepresentationParser.java
@@ -64,13 +64,11 @@ class SQLViewRepresentationParser {
}
if (!view.fieldAliases().isEmpty()) {
- JsonUtil.writeStringArray(
- SQLViewRepresentationParser.FIELD_ALIASES, view.fieldAliases(), generator);
+ JsonUtil.writeStringArray(FIELD_ALIASES, view.fieldAliases(), generator);
}
if (!view.fieldComments().isEmpty()) {
- JsonUtil.writeStringArray(
- SQLViewRepresentationParser.FIELD_COMMENTS, view.fieldComments(), generator);
+ JsonUtil.writeStringArray(FIELD_COMMENTS, view.fieldComments(), generator);
}
generator.writeEndObject();
diff --git a/core/src/main/java/org/apache/iceberg/view/ViewRepresentationParser.java b/core/src/main/java/org/apache/iceberg/view/ViewRepresentationParser.java
index 206efebc6f..79d50701ea 100644
--- a/core/src/main/java/org/apache/iceberg/view/ViewRepresentationParser.java
+++ b/core/src/main/java/org/apache/iceberg/view/ViewRepresentationParser.java
@@ -33,14 +33,15 @@ class ViewRepresentationParser {
static void toJson(ViewRepresentation representation, JsonGenerator generator)
throws IOException {
Preconditions.checkArgument(representation != null, "Invalid view representation: null");
- switch (representation.type()) {
+ switch (representation.type().toLowerCase(Locale.ENGLISH)) {
case ViewRepresentation.Type.SQL:
SQLViewRepresentationParser.toJson((SQLViewRepresentation) representation, generator);
break;
default:
- throw new IllegalArgumentException(
- String.format("Cannot serialize view representation type: %s", representation.type()));
+ throw new UnsupportedOperationException(
+ String.format(
+ "Cannot serialize unsupported view representation: %s", representation.type()));
}
}
diff --git a/core/src/main/java/org/apache/iceberg/view/ViewVersionParser.java b/core/src/main/java/org/apache/iceberg/view/ViewVersionParser.java
index ea9446df46..741647dc8d 100644
--- a/core/src/main/java/org/apache/iceberg/view/ViewVersionParser.java
+++ b/core/src/main/java/org/apache/iceberg/view/ViewVersionParser.java
@@ -24,7 +24,6 @@ import java.io.IOException;
import java.util.Map;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
-import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.util.JsonUtil;
class ViewVersionParser {
@@ -40,8 +39,10 @@ class ViewVersionParser {
static void toJson(ViewVersion version, JsonGenerator generator) throws IOException {
Preconditions.checkArgument(version != null, "Cannot serialize null view version");
generator.writeStartObject();
+
generator.writeNumberField(VERSION_ID, version.versionId());
generator.writeNumberField(TIMESTAMP_MS, version.timestampMillis());
+
generator.writeObjectFieldStart(SUMMARY);
generator.writeStringField(OPERATION, version.operation());
for (Map.Entry<String, String> summaryEntry : version.summary().entrySet()) {
@@ -49,15 +50,14 @@ class ViewVersionParser {
generator.writeStringField(summaryEntry.getKey(), summaryEntry.getValue());
}
}
-
generator.writeEndObject();
generator.writeArrayFieldStart(REPRESENTATIONS);
for (ViewRepresentation representation : version.representations()) {
ViewRepresentationParser.toJson(representation, generator);
}
-
generator.writeEndArray();
+
generator.writeEndObject();
}
@@ -80,22 +80,7 @@ class ViewVersionParser {
long timestamp = JsonUtil.getLong(TIMESTAMP_MS, node);
Map<String, String> summary = JsonUtil.getStringMap(SUMMARY, node);
Preconditions.checkArgument(
- summary != null, "Cannot parse view version with missing required field: %s", SUMMARY);
-
- String operation = null;
- ImmutableMap.Builder<String, String> versionSummary = ImmutableMap.builder();
- for (Map.Entry<String, String> summaryEntry : summary.entrySet()) {
- if (summaryEntry.getKey().equals(OPERATION)) {
- operation = summaryEntry.getValue();
- } else {
- versionSummary.put(summaryEntry.getKey(), summaryEntry.getValue());
- }
- }
-
- Preconditions.checkArgument(
- operation != null,
- "Cannot parse view version summary with missing required field: %s",
- OPERATION);
+ summary.containsKey(OPERATION), "Invalid view version summary, missing %s", OPERATION);
JsonNode serializedRepresentations = node.get(REPRESENTATIONS);
ImmutableList.Builder<ViewRepresentation> representations = ImmutableList.builder();
@@ -108,8 +93,7 @@ class ViewVersionParser {
return ImmutableViewVersion.builder()
.versionId(versionId)
.timestampMillis(timestamp)
- .summary(versionSummary.build())
- .operation(operation)
+ .summary(summary)
.representations(representations.build())
.build();
}
diff --git a/core/src/test/java/org/apache/iceberg/view/TestViewRepresentationParser.java b/core/src/test/java/org/apache/iceberg/view/TestViewRepresentationParser.java
index 4f1ce769e6..b45869c336 100644
--- a/core/src/test/java/org/apache/iceberg/view/TestViewRepresentationParser.java
+++ b/core/src/test/java/org/apache/iceberg/view/TestViewRepresentationParser.java
@@ -33,8 +33,8 @@ public class TestViewRepresentationParser {
ImmutableUnknownViewRepresentation.builder().type("unknown-sql-representation").build());
Assertions.assertThatThrownBy(() -> ViewRepresentationParser.toJson(unknownRepresentation))
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessage("Cannot serialize view representation type: unknown-sql-representation");
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessage("Cannot serialize unsupported view representation: unknown-sql-representation");
}
@Test
diff --git a/core/src/test/java/org/apache/iceberg/view/TestViewVersionParser.java b/core/src/test/java/org/apache/iceberg/view/TestViewVersionParser.java
index 08ce4e786e..06b2f64e0f 100644
--- a/core/src/test/java/org/apache/iceberg/view/TestViewVersionParser.java
+++ b/core/src/test/java/org/apache/iceberg/view/TestViewVersionParser.java
@@ -44,8 +44,7 @@ public class TestViewVersionParser {
.versionId(1)
.timestampMillis(12345)
.addRepresentations(firstRepresentation, secondRepresentation)
- .operation("create")
- .summary(ImmutableMap.of("user", "some-user"))
+ .summary(ImmutableMap.of("operation", "create", "user", "some-user"))
.build();
String serializedRepresentations =
@@ -81,8 +80,7 @@ public class TestViewVersionParser {
.versionId(1)
.timestampMillis(12345)
.addRepresentations(firstRepresentation, secondRepresentation)
- .summary(ImmutableMap.of("user", "some-user"))
- .operation("create")
+ .summary(ImmutableMap.of("operation", "create", "user", "some-user"))
.build();
String expectedRepresentations =
@@ -113,7 +111,7 @@ public class TestViewVersionParser {
Assertions.assertThatThrownBy(() -> ViewVersionParser.fromJson(viewVersionMissingOperation))
.isInstanceOf(IllegalArgumentException.class)
- .hasMessage("Cannot parse view version summary with missing required field: operation");
+ .hasMessage("Invalid view version summary, missing operation");
}
@Test