[#947]feat(jdbc-postgresql): support array type for PG (#2849)

### What changes were proposed in this pull request? support array type for PG ### Why are the changes needed? Fix: #947 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? IT and UT
apache · Apr 10, 2024 · 21de35b · 21de35b
1 parent 1b72a55
commit 21de35b
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 4 deletions.
diff --git a/...n/java/com/datastrato/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java b/...n/java/com/datastrato/gravitino/catalog/postgresql/converter/PostgreSqlTypeConverter.java
@@ -7,6 +7,9 @@
 import com.datastrato.gravitino.catalog.jdbc.converter.JdbcTypeConverter;
 import com.datastrato.gravitino.rel.types.Type;
 import com.datastrato.gravitino.rel.types.Types;
+import com.datastrato.gravitino.rel.types.Types.ListType;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 
 public class PostgreSqlTypeConverter extends JdbcTypeConverter<String> {
 
@@ -21,12 +24,16 @@ public class PostgreSqlTypeConverter extends JdbcTypeConverter<String> {
   static final String NUMERIC = "numeric";
   static final String BPCHAR = "bpchar";
   static final String BYTEA = "bytea";
+  @VisibleForTesting static final String JDBC_ARRAY_PREFIX = "_";
+  @VisibleForTesting static final String ARRAY_TOKEN = "[]";
 
   @Override
   public Type toGravitinoType(JdbcTypeBean typeBean) {
-    // TODO #947 Complex types are not considered for support in this issue, which will bring more
-    // testing needs
-    switch (typeBean.getTypeName().toLowerCase()) {
+    String typeName = typeBean.getTypeName().toLowerCase();
+    if (typeName.startsWith(JDBC_ARRAY_PREFIX)) {
+      return toGravitinoArrayType(typeName);
+    }
+    switch (typeName) {
       case BOOL:
         return Types.BooleanType.get();
       case INT_2:
@@ -100,9 +107,34 @@ public String fromGravitinoType(Type type) {
       return BPCHAR + "(" + ((Types.FixedCharType) type).length() + ")";
     } else if (type instanceof Types.BinaryType) {
       return BYTEA;
+    } else if (type instanceof Types.ListType) {
+      return fromGravitinoArrayType((ListType) type);
     }
     throw new IllegalArgumentException(
         String.format(
             "Couldn't convert Gravitino type %s to PostgreSQL type", type.simpleString()));
   }
+
+  // PG doesn't support the multidimensional array internally. The current implementation does not
+  // enforce the declared number of dimensions either. Arrays of a particular element type are all
+  // considered to be of the same type, regardless of size or number of dimensions. So, declaring
+  // the array size or number of dimensions in CREATE TABLE is simply documentation; it does not
+  // affect run-time behavior.
+  // https://www.postgresql.org/docs/current/arrays.html#ARRAYS-DECLARATION
+  private String fromGravitinoArrayType(ListType listType) {
+    Type elementType = listType.elementType();
+    Preconditions.checkArgument(
+        !listType.elementNullable(), "PostgreSQL doesn't support element to nullable");
+    Preconditions.checkArgument(
+        !(elementType instanceof ListType),
+        "PostgreSQL doesn't support multidimensional list internally, please use one dimensional list");
+    String elementTypeString = fromGravitinoType(elementType);
+    return elementTypeString + ARRAY_TOKEN;
+  }
+
+  private ListType toGravitinoArrayType(String typeName) {
+    String elementTypeName = typeName.substring(JDBC_ARRAY_PREFIX.length(), typeName.length());
+    JdbcTypeBean bean = new JdbcTypeBean(elementTypeName);
+    return ListType.of(toGravitinoType(bean), false);
+  }
 }
diff --git a/...va/com/datastrato/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java b/...va/com/datastrato/gravitino/catalog/postgresql/converter/TestPostgreSqlTypeConverter.java
@@ -9,6 +9,7 @@
 import static com.datastrato.gravitino.catalog.jdbc.converter.JdbcTypeConverter.TIME;
 import static com.datastrato.gravitino.catalog.jdbc.converter.JdbcTypeConverter.TIMESTAMP;
 import static com.datastrato.gravitino.catalog.jdbc.converter.JdbcTypeConverter.VARCHAR;
+import static com.datastrato.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.ARRAY_TOKEN;
 import static com.datastrato.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.BOOL;
 import static com.datastrato.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.BPCHAR;
 import static com.datastrato.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.BYTEA;
@@ -17,6 +18,7 @@
 import static com.datastrato.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.INT_2;
 import static com.datastrato.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.INT_4;
 import static com.datastrato.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.INT_8;
+import static com.datastrato.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.JDBC_ARRAY_PREFIX;
 import static com.datastrato.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter.NUMERIC;
 
 import com.datastrato.gravitino.catalog.jdbc.converter.JdbcTypeConverter;
@@ -52,6 +54,27 @@ public void testToGravitinoType() {
         Types.UnparsedType.of(USER_DEFINED_TYPE), USER_DEFINED_TYPE, null, null);
   }
 
+  @Test
+  public void testArrayType() {
+    Type elmentType = Types.IntegerType.get();
+    Type list1 = Types.ListType.of(elmentType, false);
+
+    checkGravitinoTypeToJdbcType(INT_4 + ARRAY_TOKEN, list1);
+    checkJdbcTypeToGravitinoType(list1, JDBC_ARRAY_PREFIX + INT_4, null, null);
+
+    // not support element nullable
+    Assertions.assertThrowsExactly(
+        IllegalArgumentException.class,
+        () ->
+            checkGravitinoTypeToJdbcType(INT_4 + ARRAY_TOKEN, Types.ListType.of(elmentType, true)));
+
+    // not support multidimensional
+    Type list2 = Types.ListType.of(list1, false);
+    Assertions.assertThrowsExactly(
+        IllegalArgumentException.class,
+        () -> checkGravitinoTypeToJdbcType(INT_4 + ARRAY_TOKEN, list2));
+  }
+
   @Test
   public void testFromGravitinoType() {
     checkGravitinoTypeToJdbcType(BOOL, Types.BooleanType.get());

diff --git a/...ava/com/datastrato/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java b/...ava/com/datastrato/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
@@ -38,6 +38,7 @@
 import com.datastrato.gravitino.rel.indexes.Indexes;
 import com.datastrato.gravitino.rel.types.Decimal;
 import com.datastrato.gravitino.rel.types.Types;
+import com.datastrato.gravitino.rel.types.Types.IntegerType;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import java.io.IOException;
@@ -238,6 +239,33 @@ private Column[] columnsWithDefaultValue() {
     };
   }
 
+  @Test
+  void testCreateTableWithArrayType() {
+    String tableName = GravitinoITUtils.genRandomName("postgresql_it_array_table");
+    Column col = Column.of("array", Types.ListType.of(IntegerType.get(), false), "col_4_comment");
+    Column[] columns = new Column[] {col};
+
+    NameIdentifier tableIdentifier =
+        NameIdentifier.of(metalakeName, catalogName, schemaName, tableName);
+
+    TableCatalog tableCatalog = catalog.asTableCatalog();
+    Table createdTable =
+        tableCatalog.createTable(tableIdentifier, columns, null, ImmutableMap.of());
+
+    Assertions.assertEquals(tableName, createdTable.name());
+    Assertions.assertEquals(columns.length, createdTable.columns().length);
+    for (int i = 0; i < columns.length; i++) {
+      assertColumn(columns[i], createdTable.columns()[i]);
+    }
+
+    Table loadTable = tableCatalog.loadTable(tableIdentifier);
+    Assertions.assertEquals(tableName, loadTable.name());
+    Assertions.assertEquals(columns.length, loadTable.columns().length);
+    for (int i = 0; i < columns.length; i++) {
+      assertColumn(columns[i], loadTable.columns()[i]);
+    }
+  }
+
   @Test
   void testCreateTableWithSpecialColumnNames() {
     // Create table from Gravitino API

diff --git a/...datastrato/gravitino/catalog/postgresql/integration/test/PostgreSqlTableOperationsIT.java b/...datastrato/gravitino/catalog/postgresql/integration/test/PostgreSqlTableOperationsIT.java
@@ -336,6 +336,12 @@ public void testCreateAllTypeTable() {
             .withType(Types.BinaryType.get())
             .withNullable(false)
             .build());
+    columns.add(
+        JdbcColumn.builder()
+            .withName("col_16")
+            .withType(Types.ListType.of(Types.IntegerType.get(), false))
+            .withNullable(true)
+            .build());
 
     // create table
     TABLE_OPERATIONS.create(

diff --git a/docs/jdbc-postgresql-catalog.md b/docs/jdbc-postgresql-catalog.md
@@ -105,9 +105,10 @@ Please refer to [Manage Relational Metadata Using Gravitino](./manage-relational
 | `VarChar`      | `Varchar`       |
 | `FixedChar`    | `Bpchar`        |
 | `Binary`       | `Bytea`         |
+| `List`         | `Array`         |
 
 :::info
-PostgreSQL doesn't support Gravitino `Fixed` `Struct` `List` `Map` `IntervalDay` `IntervalYear` `Union` `UUID` type.
+PostgreSQL doesn't support Gravitino `Fixed` `Struct` `Map` `IntervalDay` `IntervalYear` `Union` `UUID` type.
 Meanwhile, the data types other than listed above are mapped to Gravitino **[Unparsed Type](./manage-relational-metadata-using-gravitino.md#unparsed-type)** that represents an unresolvable data type since 0.5.0.
 :::