[#1632] fix(jdbc-mysql): Supporting create not null column for MySQL …

…table. (#1637) ### What changes were proposed in this pull request? Add `NOT NULL` key word in create table sentence if column type is NOT NULL. ### Why are the changes needed? We should support creating a table with not null columns. Fix: #1632 ### Does this PR introduce _any_ user-facing change? N/A. ### How was this patch tested? Add some ITs.
apache · Jan 23, 2024 · 28cb2f7 · 28cb2f7
1 parent 00b4119
commit 28cb2f7
Show file tree

Hide file tree

Showing 5 changed files with 119 additions and 7 deletions.
diff --git a/.../src/main/java/com/datastrato/gravitino/catalog/mysql/operation/MysqlTableOperations.java b/.../src/main/java/com/datastrato/gravitino/catalog/mysql/operation/MysqlTableOperations.java
@@ -308,8 +308,7 @@ protected String generateAlterTableSql(
         throw new IllegalArgumentException("Remove property is not supported yet");
       } else if (change instanceof TableChange.AddColumn) {
         TableChange.AddColumn addColumn = (TableChange.AddColumn) change;
-        lazyLoadCreateTable = getOrCreateTable(databaseName, tableName, lazyLoadCreateTable);
-        alterSql.add(addColumnFieldDefinition(addColumn, lazyLoadCreateTable));
+        alterSql.add(addColumnFieldDefinition(addColumn));
       } else if (change instanceof TableChange.RenameColumn) {
         lazyLoadCreateTable = getOrCreateTable(databaseName, tableName, lazyLoadCreateTable);
         TableChange.RenameColumn renameColumn = (TableChange.RenameColumn) change;
@@ -435,8 +434,7 @@ private String updateColumnCommentFieldDefinition(
     return "MODIFY COLUMN " + col + appendColumnDefinition(updateColumn, new StringBuilder());
   }
 
-  private String addColumnFieldDefinition(
-      TableChange.AddColumn addColumn, CreateTable createTable) {
+  private String addColumnFieldDefinition(TableChange.AddColumn addColumn) {
     String dataType = (String) typeConverter.fromGravitinoType(addColumn.getDataType());
     if (addColumn.fieldName().length > 1) {
       throw new UnsupportedOperationException("Mysql does not support nested column names.");
@@ -445,6 +443,11 @@ private String addColumnFieldDefinition(
 
     StringBuilder columnDefinition = new StringBuilder();
     columnDefinition.append("ADD COLUMN ").append(col).append(SPACE).append(dataType).append(SPACE);
+
+    if (!addColumn.isNullable()) {
+      columnDefinition.append("NOT NULL ");
+    }
+
     // Append comment if available
     if (StringUtils.isNotEmpty(addColumn.getComment())) {
       columnDefinition.append("COMMENT '").append(addColumn.getComment()).append("' ");

diff --git a/...-test/src/test/java/com/datastrato/gravitino/integration/test/trino/TrinoConnectorIT.java b/...-test/src/test/java/com/datastrato/gravitino/integration/test/trino/TrinoConnectorIT.java
@@ -1147,6 +1147,85 @@ void testMySQLTableCreatedByGravitino() throws InterruptedException {
     }
   }
 
+  @Test
+  void testMySQLTableCreatedByTrino() throws InterruptedException {
+    String catalogName = GravitinoITUtils.genRandomName("mysql_catalog").toLowerCase();
+    String schemaName = GravitinoITUtils.genRandomName("mysql_schema").toLowerCase();
+    String tableName = GravitinoITUtils.genRandomName("mysql_table").toLowerCase();
+    GravitinoMetaLake createdMetalake = client.loadMetalake(NameIdentifier.of(metalakeName));
+    String[] command = {
+      "mysql",
+      "-h127.0.0.1",
+      "-uroot",
+      "-pds123", // username and password are referred from Hive dockerfile.
+      "-e",
+      "grant all privileges on *.* to root@'%' identified by 'ds123'"
+    };
+
+    // There exists a mysql instance in Hive the container.
+    containerSuite.getHiveContainer().executeInContainer(command);
+    String hiveHost = containerSuite.getHiveContainer().getContainerIpAddress();
+
+    createdMetalake.createCatalog(
+        NameIdentifier.of(metalakeName, catalogName),
+        Catalog.Type.RELATIONAL,
+        "jdbc-mysql",
+        "comment",
+        ImmutableMap.<String, String>builder()
+            .put("jdbc-driver", "com.mysql.cj.jdbc.Driver")
+            .put("jdbc-user", "root")
+            .put("jdbc-password", "ds123")
+            .put("jdbc-url", String.format("jdbc:mysql://%s:3306?useSSL=false", hiveHost))
+            .build());
+    Catalog catalog = createdMetalake.loadCatalog(NameIdentifier.of(metalakeName, catalogName));
+    Assertions.assertEquals("root", catalog.properties().get("jdbc-user"));
+
+    String sql = String.format("show catalogs like '%s.%s'", metalakeName, catalogName);
+    boolean success = checkTrinoHasLoaded(sql, 30);
+    if (!success) {
+      Assertions.fail("Trino fail to load catalogs created by gravitino: " + sql);
+    }
+
+    String data = containerSuite.getTrinoContainer().executeQuerySQL(sql).get(0).get(0);
+    Assertions.assertEquals(metalakeName + "." + catalogName, data);
+
+    // Create schema
+    sql = String.format("create schema \"%s.%s\".%s", metalakeName, catalogName, schemaName);
+    containerSuite.getTrinoContainer().executeUpdateSQL(sql);
+
+    // create table
+    sql =
+        String.format(
+            "create table \"%s.%s\".%s.%s (id int, name varchar)",
+            metalakeName, catalogName, schemaName, tableName);
+    containerSuite.getTrinoContainer().executeUpdateSQL(sql);
+
+    // Add a not null column
+    sql =
+        String.format(
+            "alter table \"%s.%s\".%s.%s add column age int not null comment 'age of users'",
+            metalakeName, catalogName, schemaName, tableName);
+    containerSuite.getTrinoContainer().executeUpdateSQL(sql);
+
+    sql =
+        String.format(
+            "alter table \"%s.%s\".%s.%s add column address varchar(20) not null comment 'address of users'",
+            metalakeName, catalogName, schemaName, tableName);
+    containerSuite.getTrinoContainer().executeUpdateSQL(sql);
+
+    catalog
+        .asTableCatalog()
+        .loadTable(NameIdentifier.of(metalakeName, catalogName, schemaName, tableName));
+
+    sql =
+        String.format(
+            "show create table \"%s.%s\".%s.%s", metalakeName, catalogName, schemaName, tableName);
+
+    data = containerSuite.getTrinoContainer().executeQuerySQL(sql).get(0).get(0);
+    Assertions.assertTrue(data.contains("age integer NOT NULL"));
+    Assertions.assertTrue(data.contains("address varchar(20) NOT NULL"));
+  }
+
   @Test
   void testDropCatalogAndCreateAgain() throws InterruptedException {
     String catalogName = GravitinoITUtils.genRandomName("mysql_catalog").toLowerCase();

diff --git a/...ration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00002_alter_table.sql b/...ration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00002_alter_table.sql
@@ -21,6 +21,12 @@ show create table "test.gt_mysql".gt_db1.tb01;
 alter table "test.gt_mysql".gt_db1.tb01 add column city varchar(50) comment 'aaa';
 show create table "test.gt_mysql".gt_db1.tb01;
 
+alter table "test.gt_mysql".gt_db1.tb01 add column age int not null comment 'age of users';
+show create table "test.gt_mysql".gt_db1.tb01;
+
+alter table "test.gt_mysql".gt_db1.tb01 add column address varchar(200) not null comment 'address of users';
+show create table "test.gt_mysql".gt_db1.tb01;
+
 drop table "test.gt_mysql".gt_db1.tb01;
 
 drop schema "test.gt_mysql".gt_db1;
diff --git a/...ration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00002_alter_table.txt b/...ration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00002_alter_table.txt
@@ -35,6 +35,27 @@ ADD COLUMN
 )
 COMMENT ''"
 
+ADD COLUMN
+
+"CREATE TABLE ""test.gt_mysql"".gt_db1.tb01 (
+   name varchar(200),
+   salary bigint,
+   city varchar(50) COMMENT 'aaa',
+   age integer NOT NULL COMMENT 'age of users'
+)
+COMMENT ''"
+
+ADD COLUMN
+
+"CREATE TABLE ""test.gt_mysql"".gt_db1.tb01 (
+   name varchar(200),
+   salary bigint,
+   city varchar(50) COMMENT 'aaa',
+   age integer NOT NULL COMMENT 'age of users',
+   address varchar(200) NOT NULL COMMENT 'address of users'
+)
+COMMENT ''"
+
 DROP TABLE
 
 DROP SCHEMA

diff --git a/.../main/java/com/datastrato/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java b/.../main/java/com/datastrato/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java
@@ -187,7 +187,7 @@ public void renameSchema(String source, String target) {
     throw new NotImplementedException();
   }
 
-  private void applyAlter(SchemaTableName tableName, TableChange change) {
+  private void applyAlter(SchemaTableName tableName, TableChange... change) {
     try {
       tableCatalog.alterTable(
           NameIdentifier.ofTable(
@@ -233,11 +233,14 @@ public void setTableProperties(SchemaTableName schemaTableName, Map<String, Stri
   public void addColumn(SchemaTableName schemaTableName, GravitinoColumn column) {
     String[] columnNames = {column.getName()};
     if (Strings.isNullOrEmpty(column.getComment()))
-      applyAlter(schemaTableName, TableChange.addColumn(columnNames, column.getType()));
+      applyAlter(
+          schemaTableName,
+          TableChange.addColumn(columnNames, column.getType(), column.isNullable()));
     else {
       applyAlter(
           schemaTableName,
-          TableChange.addColumn(columnNames, column.getType(), column.getComment()));
+          TableChange.addColumn(
+              columnNames, column.getType(), column.getComment(), column.isNullable()));
     }
   }