Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

besides trino test, could you add a mysql test?

String dataType = (String) typeConverter.fromGravitinoType(addColumn.getDataType());
if (addColumn.fieldName().length > 1) {
throw new UnsupportedOperationException("Mysql does not support nested column names.");
Expand All @@ -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("' ");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,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"));
}

yuqi1129 marked this conversation as resolved.
Show resolved Hide resolved
@Test
void testDropCatalogAndCreateAgain() throws InterruptedException {
String catalogName = GravitinoITUtils.genRandomName("mysql_catalog").toLowerCase();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,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(
Expand Down Expand Up @@ -230,11 +230,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()));
}
}

Expand Down
Loading