Views: Fix SQL view representation field name (apache#7417)

InvisibleProgrammer · Apr 25, 2023 · 973676a · 973676a
1 parent 8459fa0
commit 973676a
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 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
@@ -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
@@ -64,13 +64,11 @@ static void toJson(SQLViewRepresentation view, JsonGenerator generator) throws I
     }
 
     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
@@ -33,14 +33,15 @@ private 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
@@ -24,7 +24,6 @@
 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,24 +39,25 @@ private 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()) {
       if (!summaryEntry.getKey().equals(OPERATION)) {
         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 @@ static ViewVersion fromJson(JsonNode node) {
     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 @@ static ViewVersion fromJson(JsonNode node) {
     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
@@ -33,8 +33,8 @@ public void testParseUnknownViewRepresentation() {
         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
@@ -44,8 +44,7 @@ public void testParseViewVersion() {
             .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 void testSerializeViewVersion() {
             .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 void testFailParsingMissingOperation() {
 
     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