Skip to content

Commit

Permalink
[#1717] fix(jdbc-mysql): throw an exception if setting comment on MyS…
Browse files Browse the repository at this point in the history
…QL database (#1784)

### What changes were proposed in this pull request?
throw a exception when user. set comment on MySQL database

### Why are the changes needed?

Fix: #1717 #1767 

### Does this PR introduce _any_ user-facing change?
yes, if user  set comment on MySQL database, will throw a exception.

### How was this patch tested?
add UT
  • Loading branch information
FANNG1 authored Jan 30, 2024
1 parent 1895e98 commit 9215c37
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
package com.datastrato.gravitino.catalog.mysql.operation;

import com.datastrato.gravitino.StringIdentifier;
import com.datastrato.gravitino.catalog.jdbc.JdbcSchema;
import com.datastrato.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations;
import com.datastrato.gravitino.exceptions.NoSuchSchemaException;
Expand Down Expand Up @@ -39,9 +40,10 @@ public class MysqlDatabaseOperations extends JdbcDatabaseOperations {
@Override
public String generateCreateDatabaseSql(
String databaseName, String comment, Map<String, String> properties) {
if (StringUtils.isNotEmpty(comment)) {
LOG.warn(
"Ignoring comment option on database create. mysql does not support comment option on database create.");
String originComment = StringIdentifier.removeIdFromComment(comment);
if (StringUtils.isNotEmpty(originComment)) {
throw new UnsupportedOperationException(
"MySQL doesn't support set schema comment: " + originComment);
}
StringBuilder sqlBuilder = new StringBuilder("CREATE DATABASE ");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void testAuditSchema() throws Exception {
Catalog catalog = createCatalog(catalogName);
NameIdentifier ident = NameIdentifier.of(metalakeName, catalogName, schemaName);
Map<String, String> prop = Maps.newHashMap();
Schema schema = catalog.asSchemas().createSchema(ident, "comment", prop);
Schema schema = catalog.asSchemas().createSchema(ident, null, prop);
Assertions.assertEquals(expectUser, schema.auditInfo().creator());
Assertions.assertNull(schema.auditInfo().lastModifier());
}
Expand All @@ -126,8 +126,7 @@ public void testAuditTable() throws Exception {

catalog
.asSchemas()
.createSchema(
NameIdentifier.of(metalakeName, catalogName, schemaName), "comment", properties);
.createSchema(NameIdentifier.of(metalakeName, catalogName, schemaName), null, properties);
Table table =
catalog
.asTableCatalog()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ public class CatalogMysqlIT extends AbstractIT {
public String alertTableName = "alert_table_name";
public String table_comment = "table_comment";

public String schema_comment = "schema_comment";
// MySQL doesn't support schema comment
public String schema_comment = null;
public String MYSQL_COL_NAME1 = "mysql_col_name1";
public String MYSQL_COL_NAME2 = "mysql_col_name2";
public String MYSQL_COL_NAME3 = "mysql_col_name3";
Expand Down Expand Up @@ -545,7 +546,7 @@ void testDropMySQLDatabase() {
.asSchemas()
.createSchema(
NameIdentifier.of(metalakeName, catalogName, schemaName),
"Created by gravitino client",
null,
ImmutableMap.<String, String>builder().build());

catalog
Expand Down Expand Up @@ -805,4 +806,23 @@ public void testAutoIncrement() {
runtimeException.getMessage(),
"Only one column can be auto-incremented. There are multiple auto-increment columns in your table: [col_1,col_6]"));
}

void testSchemaComment() {
String testSchemaName = "test";
NameIdentifier identer = NameIdentifier.of(metalakeName, catalogName, testSchemaName);
RuntimeException exception =
Assertions.assertThrowsExactly(
RuntimeException.class,
() -> catalog.asSchemas().createSchema(identer, "comment", null));
Assertions.assertTrue(
exception.getMessage().contains("MySQL doesn't support set schema comment: comment"));

// test null comment
testSchemaName = "test2";
NameIdentifier ident = NameIdentifier.of(metalakeName, catalogName, testSchemaName);
Schema schema = catalog.asSchemas().createSchema(ident, "", null);
Assertions.assertTrue(StringUtils.isEmpty(schema.comment()));
schema = catalog.asSchemas().loadSchema(ident);
Assertions.assertTrue(StringUtils.isEmpty(schema.comment()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.datanucleus.util.StringUtils;
import org.apache.commons.lang3.StringUtils;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,7 @@ void testMySQLTableCreatedByGravitino() throws InterruptedException {
.asSchemas()
.createSchema(
NameIdentifier.of(metalakeName, catalogName, schemaName),
"Created by gravitino client",
null,
ImmutableMap.<String, String>builder().build());

Assertions.assertNotNull(schema);
Expand Down

0 comments on commit 9215c37

Please sign in to comment.