[apache#2117] improvement(api): Type adds UNPARSED column data type t…

…o handle an unresolvable type from the catalog
SteNicholas · Feb 22, 2024 · 8000226 · 8000226
1 parent 15e8a4e
commit 8000226
Show file tree

Hide file tree

Showing 14 changed files with 176 additions and 19 deletions.
diff --git a/api/src/main/java/com/datastrato/gravitino/rel/types/Type.java b/api/src/main/java/com/datastrato/gravitino/rel/types/Type.java
@@ -73,7 +73,10 @@ enum Name {
     UNION,
 
     /** The null type. A null type represents a value that is null. */
-    NULL
+    NULL,
+
+    /** The unparsed type. An unparsed type represents an unresolvable type. */
+    UNPARSED
   }
 
   /** The base type of all primitive types. */

diff --git a/api/src/main/java/com/datastrato/gravitino/rel/types/Types.java b/api/src/main/java/com/datastrato/gravitino/rel/types/Types.java
@@ -1017,6 +1017,61 @@ public int hashCode() {
     }
   }
 
+  /**
+   * Represents a type that is not parsed yet. The parsed type is represented by other types of
+   * {@link Types}.
+   */
+  public static class UnparsedType implements Type {
+
+    /**
+     * Creates a new {@link UnparsedType} with the given unparsed type.
+     *
+     * @param unparsedType The unparsed type.
+     * @return A new {@link UnparsedType} with the given unparsed type.
+     */
+    public static UnparsedType of(String unparsedType) {
+      return new UnparsedType(unparsedType);
+    }
+
+    private final String unparsedType;
+
+    private UnparsedType(String unparsedType) {
+      this.unparsedType = unparsedType;
+    }
+
+    /** @return The unparsed type as a string. */
+    public String unparsedType() {
+      return unparsedType;
+    }
+
+    @Override
+    public Name name() {
+      return Name.UNPARSED;
+    }
+
+    @Override
+    public String simpleString() {
+      return String.format("unparsed(%s)", unparsedType);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) {
+        return true;
+      }
+      if (o == null || getClass() != o.getClass()) {
+        return false;
+      }
+      UnparsedType that = (UnparsedType) o;
+      return Objects.equals(unparsedType, that.unparsedType);
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(unparsedType);
+    }
+  }
+
   /**
    * @param dataType The data type to check.
    * @return True if the given data type is allowed to be an auto-increment column.

diff --git a/api/src/test/java/com/datastrato/gravitino/rel/TestTypes.java b/api/src/test/java/com/datastrato/gravitino/rel/TestTypes.java
@@ -179,4 +179,13 @@ public void testComplexTypes() {
     Assertions.assertEquals("union<integer,string,boolean>", unionType.simpleString());
     Assertions.assertEquals(unionType, Types.UnionType.of(unionType.types()));
   }
+
+  @Test
+  public void testUnparsedType() {
+    Types.UnparsedType unparsedType = Types.UnparsedType.of("bit");
+    Assertions.assertEquals(Type.Name.UNPARSED, unparsedType.name());
+    Assertions.assertEquals("unparsed(bit)", unparsedType.simpleString());
+    Assertions.assertEquals("bit", unparsedType.unparsedType());
+    Assertions.assertEquals(unparsedType, Types.UnparsedType.of("bit"));
+  }
 }
diff --git a/...alog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/FromHiveType.java b/...alog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/converter/FromHiveType.java
@@ -21,6 +21,7 @@
 
 import com.datastrato.gravitino.rel.types.Type;
 import com.datastrato.gravitino.rel.types.Types;
+import com.google.common.annotations.VisibleForTesting;
 import java.util.ArrayList;
 import java.util.stream.IntStream;
 import org.apache.hadoop.hive.serde2.typeinfo.CharTypeInfo;
@@ -40,9 +41,8 @@ public class FromHiveType {
    *
    * @param hiveType The Hive data type string to convert.
    * @return The equivalent Gravitino data type.
-   * @throws IllegalArgumentException If the Hive data type is unknown or unsupported.
    */
-  public static Type convert(String hiveType) throws IllegalArgumentException {
+  public static Type convert(String hiveType) {
     TypeInfo hiveTypeInfo = getTypeInfoFromTypeString(hiveType);
     return toGravitinoType(hiveTypeInfo);
   }
@@ -52,9 +52,9 @@ public static Type convert(String hiveType) throws IllegalArgumentException {
    *
    * @param hiveTypeInfo The Hive TypeInfo object to convert.
    * @return The equivalent Gravitino Type.
-   * @throws IllegalArgumentException if the Hive data type category is unknown or unsupported.
    */
-  private static Type toGravitinoType(TypeInfo hiveTypeInfo) throws IllegalArgumentException {
+  @VisibleForTesting
+  public static Type toGravitinoType(TypeInfo hiveTypeInfo) {
     switch (hiveTypeInfo.getCategory()) {
       case PRIMITIVE:
         switch (hiveTypeInfo.getTypeName()) {
@@ -98,8 +98,7 @@ private static Type toGravitinoType(TypeInfo hiveTypeInfo) throws IllegalArgumen
               return Types.DecimalType.of(decimalTypeInfo.precision(), decimalTypeInfo.scale());
             }
 
-            throw new IllegalArgumentException(
-                "Unknown Hive type: " + hiveTypeInfo.getQualifiedName());
+            return Types.UnparsedType.of(hiveTypeInfo.getQualifiedName());
         }
       case LIST:
         return Types.ListType.nullable(
@@ -128,8 +127,7 @@ private static Type toGravitinoType(TypeInfo hiveTypeInfo) throws IllegalArgumen
                 .map(FromHiveType::toGravitinoType)
                 .toArray(Type[]::new));
       default:
-        throw new IllegalArgumentException(
-            "Unknown category of Hive type: " + hiveTypeInfo.getCategory());
+        return Types.UnparsedType.of(hiveTypeInfo.getQualifiedName());
     }
   }
 

diff --git a/...alog-hive/src/test/java/com/datastrato/gravitino/catalog/converter/TestTypeConverter.java b/...alog-hive/src/test/java/com/datastrato/gravitino/catalog/converter/TestTypeConverter.java
@@ -29,12 +29,17 @@
 
 import com.datastrato.gravitino.catalog.hive.converter.FromHiveType;
 import com.datastrato.gravitino.catalog.hive.converter.ToHiveType;
+import com.datastrato.gravitino.rel.types.Types;
 import java.util.Arrays;
+import org.apache.hadoop.hive.serde2.typeinfo.PrimitiveTypeInfo;
 import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
 public class TestTypeConverter {
+
+  private static final String USER_DEFINED_TYPE = "user-defined";
+
   @Test
   public void testTypeConverter() {
     testConverter(BOOLEAN_TYPE_NAME);
@@ -69,11 +74,24 @@ public void testTypeConverter() {
                 Arrays.asList(
                     getPrimitiveTypeInfo(STRING_TYPE_NAME), getPrimitiveTypeInfo(INT_TYPE_NAME)))
             .getTypeName());
+    Assertions.assertEquals(
+        Types.UnparsedType.of(USER_DEFINED_TYPE),
+        FromHiveType.toGravitinoType(new UserDefinedTypeInfo()));
+    Assertions.assertThrows(
+        UnsupportedOperationException.class,
+        () -> ToHiveType.convert(Types.UnparsedType.of(USER_DEFINED_TYPE)));
   }
 
   private void testConverter(String typeName) {
     TypeInfo hiveType = getTypeInfoFromTypeString(typeName);
     TypeInfo convertedType = ToHiveType.convert(FromHiveType.convert(hiveType.getTypeName()));
     Assertions.assertEquals(hiveType, convertedType);
   }
+
+  static class UserDefinedTypeInfo extends PrimitiveTypeInfo {
+    @Override
+    public String getTypeName() {
+      return USER_DEFINED_TYPE;
+    }
+  }
 }
diff --git a/...ql/src/main/java/com/datastrato/gravitino/catalog/mysql/converter/MysqlTypeConverter.java b/...ql/src/main/java/com/datastrato/gravitino/catalog/mysql/converter/MysqlTypeConverter.java
@@ -55,7 +55,7 @@ public Type toGravitinoType(JdbcTypeBean typeBean) {
       case BINARY:
         return Types.BinaryType.get();
       default:
-        throw new IllegalArgumentException("Not a supported type: " + typeBean.getTypeName());
+        return Types.UnparsedType.of(typeBean.getTypeName());
     }
   }
 
@@ -90,6 +90,7 @@ public String fromGravitinoType(Type type) {
     } else if (type instanceof Types.BinaryType) {
       return type.simpleString();
     }
-    throw new IllegalArgumentException("Not a supported type: " + type.toString());
+    throw new IllegalArgumentException(
+        String.format("Couldn't convert Gravitino type %s to MySQL type", type.simpleString()));
   }
 }
diff --git a/...rc/test/java/com/datastrato/gravitino/catalog/mysql/converter/TestMysqlTypeConverter.java b/...rc/test/java/com/datastrato/gravitino/catalog/mysql/converter/TestMysqlTypeConverter.java
@@ -28,6 +28,7 @@
 public class TestMysqlTypeConverter {
 
   private static final MysqlTypeConverter MYSQL_TYPE_CONVERTER = new MysqlTypeConverter();
+  private static final String USER_DEFINED_TYPE = "user-defined";
 
   @Test
   public void testToGravitinoType() {
@@ -44,6 +45,8 @@ public void testToGravitinoType() {
     checkJdbcTypeToGravitinoType(Types.FixedCharType.of(20), CHAR, "20", null);
     checkJdbcTypeToGravitinoType(Types.StringType.get(), TEXT, null, null);
     checkJdbcTypeToGravitinoType(Types.BinaryType.get(), BINARY, null, null);
+    checkJdbcTypeToGravitinoType(
+        Types.UnparsedType.of(USER_DEFINED_TYPE), USER_DEFINED_TYPE, null, null);
   }
 
   @Test
@@ -61,6 +64,9 @@ public void testFromGravitinoType() {
     checkGravitinoTypeToJdbcType(CHAR + "(20)", Types.FixedCharType.of(20));
     checkGravitinoTypeToJdbcType(TEXT, Types.StringType.get());
     checkGravitinoTypeToJdbcType(BINARY, Types.BinaryType.get());
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> MYSQL_TYPE_CONVERTER.fromGravitinoType(Types.UnparsedType.of(USER_DEFINED_TYPE)));
   }
 
   protected void checkGravitinoTypeToJdbcType(String jdbcTypeName, Type gravitinoType) {

diff --git a/...n/java/com/datastrato/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java b/...n/java/com/datastrato/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java
@@ -59,7 +59,7 @@ public Type toGravitinoType(JdbcTypeBean typeBean) {
       case BYTEA:
         return Types.BinaryType.get();
       default:
-        throw new IllegalArgumentException("Not a supported type: " + typeBean);
+        return Types.UnparsedType.of(typeBean.getTypeName());
     }
   }
 
@@ -102,6 +102,7 @@ public String fromGravitinoType(Type type) {
       return BYTEA;
     }
     throw new IllegalArgumentException(
-        String.format("Couldn't convert PostgreSQL type %s to Gravitino type", type.toString()));
+        String.format(
+            "Couldn't convert Gravitino type %s to PostgreSQL type", type.simpleString()));
   }
 }
diff --git a/...va/com/datastrato/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java b/...va/com/datastrato/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java
@@ -30,6 +30,7 @@ public class TestPostgreSqlTypeConverter {
 
   private static final PostgreSqlTypeConverter POSTGRE_SQL_TYPE_CONVERTER =
       new PostgreSqlTypeConverter();
+  private static final String USER_DEFINED_TYPE = "user-defined";
 
   @Test
   public void testToGravitinoType() {
@@ -47,6 +48,8 @@ public void testToGravitinoType() {
     checkJdbcTypeToGravitinoType(Types.FixedCharType.of(20), BPCHAR, "20", null);
     checkJdbcTypeToGravitinoType(Types.StringType.get(), TEXT, null, null);
     checkJdbcTypeToGravitinoType(Types.BinaryType.get(), BYTEA, null, null);
+    checkJdbcTypeToGravitinoType(
+        Types.UnparsedType.of(USER_DEFINED_TYPE), USER_DEFINED_TYPE, null, null);
   }
 
   @Test
@@ -65,6 +68,10 @@ public void testFromGravitinoType() {
     checkGravitinoTypeToJdbcType(BPCHAR + "(20)", Types.FixedCharType.of(20));
     checkGravitinoTypeToJdbcType(TEXT, Types.StringType.get());
     checkGravitinoTypeToJdbcType(BYTEA, Types.BinaryType.get());
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            POSTGRE_SQL_TYPE_CONVERTER.fromGravitinoType(Types.UnparsedType.of(USER_DEFINED_TYPE)));
   }
 
   protected void checkGravitinoTypeToJdbcType(String jdbcTypeName, Type gravitinoType) {

diff --git a/common/src/main/java/com/datastrato/gravitino/json/JsonUtils.java b/common/src/main/java/com/datastrato/gravitino/json/JsonUtils.java
@@ -99,6 +99,8 @@ public class JsonUtils {
   private static final String LIST = "list";
   private static final String MAP = "map";
   private static final String UNION = "union";
+  private static final String UNPARSED = "unparsed";
+  private static final String UNPARSED_TYPE = "unparsedType";
   private static final String FIELDS = "fields";
   private static final String UNION_TYPES = "types";
   private static final String STRUCT_FIELD_NAME = "name";
@@ -488,8 +490,11 @@ private static void writeDataType(Type dataType, JsonGenerator gen) throws IOExc
       case UNION:
         writeUnionType((Types.UnionType) dataType, gen);
         break;
+      case UNPARSED:
+        writeUnparsedType((Types.UnparsedType) dataType, gen);
+        break;
       default:
-        throw new IOException("Cannot serialize unknown type: " + dataType);
+        writeUnparsedType(dataType.simpleString(), gen);
     }
   }
 
@@ -510,9 +515,8 @@ private static Type readDataType(JsonNode node) {
           : fromPrimitiveTypeString(text);
     }
 
-    if (node.isObject() && node.get(TYPE) != null) {
-      JsonNode typeField = node.get(TYPE);
-      String type = typeField.asText();
+    if (node.isObject() && node.has(TYPE)) {
+      String type = node.get(TYPE).asText();
 
       if (STRUCT.equals(type)) {
         return readStructType(node);
@@ -529,6 +533,10 @@ private static Type readDataType(JsonNode node) {
       if (UNION.equals(type)) {
         return readUnionType(node);
       }
+
+      if (UNPARSED.equals(type)) {
+        return readUnparsedType(node);
+      }
     }
 
     throw new IllegalArgumentException("Cannot parse type from JSON: " + node);
@@ -599,6 +607,18 @@ private static void writeStructField(Types.StructType.Field field, JsonGenerator
     gen.writeEndObject();
   }
 
+  private static void writeUnparsedType(Types.UnparsedType unparsedType, JsonGenerator gen)
+      throws IOException {
+    writeUnparsedType(unparsedType.unparsedType(), gen);
+  }
+
+  private static void writeUnparsedType(String unparsedType, JsonGenerator gen) throws IOException {
+    gen.writeStartObject();
+    gen.writeStringField(TYPE, UNPARSED);
+    gen.writeStringField(UNPARSED_TYPE, unparsedType);
+    gen.writeEndObject();
+  }
+
   private static Type.PrimitiveType fromPrimitiveTypeString(String typeString) {
     Type.PrimitiveType primitiveType = TYPES.get(typeString);
     if (primitiveType != null) {
@@ -701,6 +721,13 @@ private static Types.StructType.Field readStructField(JsonNode node) {
     return Types.StructType.Field.of(name, type, nullable, comment);
   }
 
+  private static Types.UnparsedType readUnparsedType(JsonNode node) {
+    Preconditions.checkArgument(
+        node.has(UNPARSED_TYPE), "Cannot parse unparsed type from missing unparsed type: %s", node);
+
+    return Types.UnparsedType.of(node.get(UNPARSED_TYPE).asText());
+  }
+
   // Nested classes for custom serialization and deserialization
 
   /** Custom JSON serializer for Gravitino Type objects. */

diff --git a/common/src/test/java/com/datastrato/gravitino/json/TestJsonUtils.java b/common/src/test/java/com/datastrato/gravitino/json/TestJsonUtils.java
@@ -154,6 +154,12 @@ public void testTypeSerDe() throws Exception {
             + "    ]\n"
             + "}";
     Assertions.assertEquals(objectMapper.readTree(expected), objectMapper.readTree(jsonValue));
+
+    type = Types.UnparsedType.of("user-defined");
+    jsonValue = JsonUtils.objectMapper().writeValueAsString(type);
+    expected =
+        "{\n" + "    \"type\": \"unparsed\",\n" + "    \"unparsedType\": \"user-defined\"\n" + "}";
+    Assertions.assertEquals(objectMapper.readTree(expected), objectMapper.readTree(jsonValue));
   }
 
   @Test

diff --git a/...est/java/com/datastrato/gravitino/integration/test/catalog/jdbc/mysql/CatalogMysqlIT.java b/...est/java/com/datastrato/gravitino/integration/test/catalog/jdbc/mysql/CatalogMysqlIT.java
@@ -1225,4 +1225,16 @@ void testMySQLTableNameCaseSensitive() {
     assertionsTableInfo(
         "TABLENAME", table_comment, Arrays.asList(newColumns), properties, indexes, table);
   }
+
+  @Test
+  void testUnparsedTypeConverter() {
+    String tableName = GravitinoITUtils.genRandomName("test_unparsed_type");
+    mysqlService.executeQuery(
+        String.format("CREATE TABLE %s.%s (bit_col bit);", schemaName, tableName));
+    Table loadedTable =
+        catalog
+            .asTableCatalog()
+            .loadTable(NameIdentifier.of(metalakeName, catalogName, schemaName, tableName));
+    Assertions.assertEquals(Types.UnparsedType.of("BIT"), loadedTable.columns()[0].dataType());
+  }
 }
diff --git a/.../com/datastrato/gravitino/integration/test/catalog/jdbc/mysql/MysqlTableOperationsIT.java b/.../com/datastrato/gravitino/integration/test/catalog/jdbc/mysql/MysqlTableOperationsIT.java
@@ -605,7 +605,9 @@ public void testCreateNotSupportTypeTable() {
       Assertions.assertTrue(
           illegalArgumentException
               .getMessage()
-              .contains("Not a supported type: " + type.toString()));
+              .contains(
+                  String.format(
+                      "Couldn't convert Gravitino type %s to MySQL type", type.simpleString())));
     }
   }