yuqi1129 commented on code in PR #9826:
URL: https://github.com/apache/gravitino/pull/9826#discussion_r2767864284
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java:
##########
@@ -59,6 +63,381 @@
public class TestClickHouseTableOperations extends TestClickHouse {
private static final Type STRING = Types.StringType.get();
private static final Type INT = Types.IntegerType.get();
+ private static final Type LONG = Types.LongType.get();
+
+ @Test
+ public void testCreateAndAlterTable() {
+ String tableName = RandomStringUtils.randomAlphabetic(16) + "_op_table";
+ String tableComment = "test_comment";
+ List<JdbcColumn> columns = new ArrayList<>();
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_1")
+ .withType(STRING)
+ .withComment("test_comment")
+ .withNullable(true)
+ .build());
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_2")
+ .withType(INT)
+ .withNullable(false)
+ .withComment("set primary key")
+ .build());
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_3")
+ .withType(INT)
+ .withNullable(true)
+ .withDefaultValue(Literals.NULL)
+ .build());
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_4")
+ .withType(STRING)
+ .withDefaultValue(Literals.of("hello world", STRING))
+ .withNullable(false)
+ .build());
+ Map<String, String> properties = new HashMap<>();
+
+ Index[] indexes = new Index[] {};
+ // create table
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME.toString(),
+ tableName,
+ columns.toArray(new JdbcColumn[0]),
+ tableComment,
+ properties,
+ null,
+ Distributions.NONE,
+ indexes,
+ getSortOrders("col_2"));
+
+ // list table
+ List<String> tables = TABLE_OPERATIONS.listTables(TEST_DB_NAME.toString());
+ Assertions.assertTrue(tables.contains(tableName));
+
+ // load table
+ JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName);
+ assertionsTableInfo(
+ tableName, tableComment, columns, properties, indexes,
Transforms.EMPTY_TRANSFORM, load);
+
+ // rename table
+ String newName = "new_table";
+ Assertions.assertDoesNotThrow(
+ () -> TABLE_OPERATIONS.rename(TEST_DB_NAME.toString(), tableName,
newName));
+ Assertions.assertDoesNotThrow(() ->
TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), newName));
+
+ // alter table
+ JdbcColumn newColumn =
+ JdbcColumn.builder()
+ .withName("col_5")
+ .withType(STRING)
+ .withComment("new_add")
+ .withNullable(false) //
+ // .withDefaultValue(Literals.of("hello test", STRING))
+ .build();
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME.toString(),
+ newName,
+ TableChange.addColumn(
+ new String[] {newColumn.name()},
+ newColumn.dataType(),
+ newColumn.comment(),
+ TableChange.ColumnPosition.after("col_1"),
+ newColumn.nullable(),
+ newColumn.autoIncrement(),
+ newColumn.defaultValue())
+ // ,
+ // TableChange.setProperty(CLICKHOUSE_ENGINE_KEY, "InnoDB"));
+ // properties.put(CLICKHOUSE_ENGINE_KEY, "InnoDB"
+ );
+ load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), newName);
+ List<JdbcColumn> alterColumns =
+ new ArrayList<JdbcColumn>() {
+ {
+ add(columns.get(0));
+ add(newColumn);
+ add(columns.get(1));
+ add(columns.get(2));
+ add(columns.get(3));
+ }
+ };
+ assertionsTableInfo(
+ newName, tableComment, alterColumns, properties, indexes,
Transforms.EMPTY_TRANSFORM, load);
+
+ // Detect unsupported properties
+ TableChange setProperty = TableChange.setProperty(CLICKHOUSE_ENGINE_KEY,
"ABC");
+ UnsupportedOperationException gravitinoRuntimeException =
+ Assertions.assertThrows(
+ UnsupportedOperationException.class,
+ () -> TABLE_OPERATIONS.alterTable(TEST_DB_NAME.toString(),
newName, setProperty));
+ Assertions.assertTrue(
+ StringUtils.contains(
+ gravitinoRuntimeException.getMessage(),
+ "Alter table properties in ClickHouse is not supported"));
+
+ // delete column
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME.toString(),
+ newName,
+ TableChange.deleteColumn(new String[] {newColumn.name()}, true));
+ load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), newName);
+ assertionsTableInfo(
+ newName, tableComment, columns, properties, indexes,
Transforms.EMPTY_TRANSFORM, load);
+
+ TableChange deleteColumn = TableChange.deleteColumn(new String[]
{newColumn.name()}, false);
+ IllegalArgumentException illegalArgumentException =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () -> TABLE_OPERATIONS.alterTable(TEST_DB_NAME.toString(),
newName, deleteColumn));
+ Assertions.assertEquals(
+ "Delete column '%s' does not exist.".formatted(newColumn.name()),
+ illegalArgumentException.getMessage());
+ Assertions.assertDoesNotThrow(
+ () ->
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME.toString(),
+ newName,
+ TableChange.deleteColumn(new String[] {newColumn.name()},
true)));
+
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME.toString(),
+ newName,
+ TableChange.deleteColumn(new String[] {newColumn.name()}, true));
+ Assertions.assertTrue(
+ TABLE_OPERATIONS.drop(TEST_DB_NAME.toString(), newName), "table should
be dropped");
+
+ GravitinoRuntimeException exception =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () -> TABLE_OPERATIONS.drop(TEST_DB_NAME.toString(), newName));
+ Assertions.assertTrue(StringUtils.contains(exception.getMessage(), "does
not exist"));
+ }
+
+ @Test
+ public void testAlterTable() {
+ String tableName = RandomStringUtils.randomAlphabetic(16) + "_al_table";
+ String tableComment = "test_comment";
+ List<JdbcColumn> columns = new ArrayList<>();
+ JdbcColumn col_1 =
+ JdbcColumn.builder()
+ .withName("col_1")
+ .withType(INT)
+ .withComment("id")
+ .withNullable(false)
+ .withDefaultValue(Literals.integerLiteral(0))
+ .build();
+ columns.add(col_1);
+ JdbcColumn col_2 =
+ JdbcColumn.builder()
+ .withName("col_2")
+ .withType(STRING)
+ .withComment("name")
+ .withDefaultValue(Literals.of("hello world", STRING))
+ .withNullable(false)
+ .build();
+ columns.add(col_2);
+ JdbcColumn col_3 =
+ JdbcColumn.builder()
+ .withName("col_3")
+ .withType(STRING)
+ .withComment("name")
+ .withDefaultValue(Literals.NULL)
+ .build();
+ // `col_1` int NOT NULL COMMENT 'id' ,
+ // `col_2` STRING(255) NOT NULL DEFAULT 'hello world' COMMENT 'name' ,
+ // `col_3` STRING(255) NULL DEFAULT NULL COMMENT 'name' ,
+ columns.add(col_3);
+ Map<String, String> properties = new HashMap<>();
+
+ // create table
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME.toString(),
+ tableName,
+ columns.toArray(new JdbcColumn[0]),
+ tableComment,
+ properties,
+ null,
+ Distributions.NONE,
+ null,
+ getSortOrders("col_2"));
+ JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName);
+ assertionsTableInfo(
+ tableName, tableComment, columns, properties, null,
Transforms.EMPTY_TRANSFORM, load);
+
Review Comment:
done
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java:
##########
@@ -59,6 +63,381 @@
public class TestClickHouseTableOperations extends TestClickHouse {
private static final Type STRING = Types.StringType.get();
private static final Type INT = Types.IntegerType.get();
+ private static final Type LONG = Types.LongType.get();
+
+ @Test
+ public void testCreateAndAlterTable() {
+ String tableName = RandomStringUtils.randomAlphabetic(16) + "_op_table";
+ String tableComment = "test_comment";
+ List<JdbcColumn> columns = new ArrayList<>();
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_1")
+ .withType(STRING)
+ .withComment("test_comment")
+ .withNullable(true)
+ .build());
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_2")
+ .withType(INT)
+ .withNullable(false)
+ .withComment("set primary key")
+ .build());
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_3")
+ .withType(INT)
+ .withNullable(true)
+ .withDefaultValue(Literals.NULL)
+ .build());
+ columns.add(
+ JdbcColumn.builder()
+ .withName("col_4")
+ .withType(STRING)
+ .withDefaultValue(Literals.of("hello world", STRING))
+ .withNullable(false)
+ .build());
+ Map<String, String> properties = new HashMap<>();
+
+ Index[] indexes = new Index[] {};
+ // create table
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME.toString(),
+ tableName,
+ columns.toArray(new JdbcColumn[0]),
+ tableComment,
+ properties,
+ null,
+ Distributions.NONE,
+ indexes,
+ getSortOrders("col_2"));
+
+ // list table
+ List<String> tables = TABLE_OPERATIONS.listTables(TEST_DB_NAME.toString());
+ Assertions.assertTrue(tables.contains(tableName));
+
+ // load table
+ JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName);
+ assertionsTableInfo(
+ tableName, tableComment, columns, properties, indexes,
Transforms.EMPTY_TRANSFORM, load);
+
+ // rename table
+ String newName = "new_table";
+ Assertions.assertDoesNotThrow(
+ () -> TABLE_OPERATIONS.rename(TEST_DB_NAME.toString(), tableName,
newName));
+ Assertions.assertDoesNotThrow(() ->
TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), newName));
+
+ // alter table
Review Comment:
done
##########
catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java:
##########
@@ -327,8 +353,368 @@ protected String generatePurgeTableSql(String tableName) {
@Override
protected String generateAlterTableSql(
String databaseName, String tableName, TableChange... changes) {
- throw new UnsupportedOperationException(
- "ClickHouseTableOperations.generateAlterTableSql is not implemented
yet.");
+ // Not all operations require the original table information, so lazy
loading is used here
+ JdbcTable lazyLoadTable = null;
+ TableChange.UpdateComment updateComment = null;
+ List<TableChange.SetProperty> setProperties = new ArrayList<>();
+ List<String> alterSql = new ArrayList<>();
+
+ for (TableChange change : changes) {
+ if (change instanceof TableChange.UpdateComment) {
+ updateComment = (TableChange.UpdateComment) change;
+
+ } else if (change instanceof TableChange.SetProperty setProperty) {
+ // The set attribute needs to be added at the end.
+ setProperties.add(setProperty);
+
+ } else if (change instanceof TableChange.RemoveProperty) {
+ // Clickhouse does not support deleting table attributes, it can be
replaced by Set Property
+ throw new IllegalArgumentException("Remove property is not supported
yet");
+
+ } else if (change instanceof TableChange.AddColumn addColumn) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ alterSql.add(addColumnFieldDefinition(addColumn));
+
+ } else if (change instanceof TableChange.RenameColumn renameColumn) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ alterSql.add(renameColumnFieldDefinition(renameColumn));
+
+ } else if (change instanceof TableChange.UpdateColumnDefaultValue
updateColumnDefaultValue) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ alterSql.add(
+ updateColumnDefaultValueFieldDefinition(updateColumnDefaultValue,
lazyLoadTable));
+
+ } else if (change instanceof TableChange.UpdateColumnType
updateColumnType) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ alterSql.add(updateColumnTypeFieldDefinition(updateColumnType,
lazyLoadTable));
+
+ } else if (change instanceof TableChange.UpdateColumnComment
updateColumnComment) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ alterSql.add(updateColumnCommentFieldDefinition(updateColumnComment,
lazyLoadTable));
+
+ } else if (change instanceof TableChange.UpdateColumnPosition
updateColumnPosition) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ alterSql.add(updateColumnPositionFieldDefinition(updateColumnPosition,
lazyLoadTable));
+
+ } else if (change instanceof TableChange.DeleteColumn deleteColumn) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ String deleteColSql = deleteColumnFieldDefinition(deleteColumn,
lazyLoadTable);
+
+ if (StringUtils.isNotEmpty(deleteColSql)) {
+ alterSql.add(deleteColSql);
+ }
+
+ } else if (change instanceof TableChange.UpdateColumnNullability) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ alterSql.add(
+ updateColumnNullabilityDefinition(
+ (TableChange.UpdateColumnNullability) change, lazyLoadTable));
+
+ } else if (change instanceof TableChange.DeleteIndex) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ alterSql.add(deleteIndexDefinition(lazyLoadTable,
(TableChange.DeleteIndex) change));
+
+ } else if (change instanceof TableChange.UpdateColumnAutoIncrement) {
+ lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ alterSql.add(
+ updateColumnAutoIncrementDefinition(
+ lazyLoadTable, (TableChange.UpdateColumnAutoIncrement)
change));
+
+ } else {
+ throw new IllegalArgumentException(
+ "Unsupported table change type: " + change.getClass().getName());
+ }
+ }
+
+ // Last modified comment
+ if (null != updateComment) {
+ String newComment = updateComment.getNewComment();
+ if (null == StringIdentifier.fromComment(newComment)) {
+ // Detect and add Gravitino id.
+ JdbcTable jdbcTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
+ StringIdentifier identifier =
StringIdentifier.fromComment(jdbcTable.comment());
+ if (null != identifier) {
+ newComment = StringIdentifier.addToComment(identifier, newComment);
+ }
+ }
+ alterSql.add(" MODIFY COMMENT '%s'".formatted(newComment));
+ }
+
+ if (!setProperties.isEmpty()) {
+ alterSql.add(generateAlterTableProperties(setProperties));
+ }
+
+ // Remove all empty SQL statements
+ List<String> nonEmptySQLs =
+
alterSql.stream().filter(StringUtils::isNotEmpty).collect(Collectors.toList());
+ if (CollectionUtils.isEmpty(nonEmptySQLs)) {
+ return "";
+ }
+
+ // Return the generated SQL statement
+ String result =
+ "ALTER TABLE %s \n%s;"
+ .formatted(quoteIdentifier(tableName), String.join(",\n",
nonEmptySQLs));
+ LOG.info("Generated alter table:{} sql: {}", databaseName + "." +
tableName, result);
+ return result;
+ }
+
+ private String updateColumnAutoIncrementDefinition(
+ JdbcTable table, TableChange.UpdateColumnAutoIncrement change) {
+ if (change.fieldName().length > 1) {
+ throw new UnsupportedOperationException("Nested column names are not
supported");
+ }
+
+ String col = change.fieldName()[0];
+ JdbcColumn column = getJdbcColumnFromTable(table, col);
+ if (change.isAutoIncrement()) {
+ Preconditions.checkArgument(
+ Types.allowAutoIncrement(column.dataType()),
+ "Auto increment is not allowed, type: " + column.dataType());
+ }
+
+ JdbcColumn updateColumn =
+ JdbcColumn.builder()
+ .withName(col)
+ .withDefaultValue(column.defaultValue())
+ .withNullable(column.nullable())
+ .withType(column.dataType())
+ .withComment(column.comment())
+ .withAutoIncrement(change.isAutoIncrement())
+ .build();
+
+ return MODIFY_COLUMN
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
