zhoujinsong commented on code in PR #3167:
URL: https://github.com/apache/amoro/pull/3167#discussion_r1753336967
##########
amoro-ams/src/main/java/org/apache/amoro/server/persistence/mapper/TableMetaMapper.java:
##########
@@ -244,7 +257,7 @@ TableMetadata selectTableMetaByName(
@Insert(
"INSERT INTO table_identifier(catalog_name, db_name, table_name, format)
VALUES("
+ " #{tableIdentifier.catalog}, #{tableIdentifier.database},
#{tableIdentifier.tableName}, "
- + " #{tableIdentifier.format})")
+ + " #{tableIdentifier.format,
typeHandler=org.apache.amoro.server.persistence.converter.TableFormatConverter})")
Review Comment:
It seems the full class name is not necessary here.
##########
amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/TableController.java:
##########
@@ -497,18 +497,14 @@ public void getTableList(Context ctx) {
ServerCatalog serverCatalog = tableService.getServerCatalog(catalog);
Function<TableFormat, String> formatToType =
format -> {
- switch (format) {
- case MIXED_HIVE:
- case MIXED_ICEBERG:
- return TableMeta.TableType.ARCTIC.toString();
- case PAIMON:
- return TableMeta.TableType.PAIMON.toString();
- case ICEBERG:
- return TableMeta.TableType.ICEBERG.toString();
- case HUDI:
- return TableMeta.TableType.HUDI.toString();
- default:
- throw new IllegalStateException("Unknown format");
+ if (format.equals(TableFormat.MIXED_HIVE) ||
format.equals(TableFormat.MIXED_ICEBERG)) {
+ return TableMeta.TableType.ARCTIC.toString();
+ } else if (format.equals(TableFormat.PAIMON)) {
+ return TableMeta.TableType.PAIMON.toString();
+ } else if (format.equals(TableFormat.ICEBERG)) {
Review Comment:
It seems that the processing of Iceberg and Hudi has been mixed together
here.
##########
amoro-format-iceberg/src/test/java/org/apache/amoro/catalog/TableTestBase.java:
##########
@@ -44,14 +45,11 @@ public void setupTable() {
this.tableMetaStore =
MixedFormatCatalogUtil.buildMetaStore(getCatalogMeta());
getUnifiedCatalog().createDatabase(TableTestHelper.TEST_DB_NAME);
- switch (getTestFormat()) {
- case MIXED_HIVE:
- case MIXED_ICEBERG:
- createMixedFormatTable();
- break;
- case ICEBERG:
- createIcebergFormatTable();
- break;
+ TableFormat format = getTestFormat();
+ if (TableFormat.MIXED_HIVE.equals(format) ||
TableFormat.MIXED_ICEBERG.equals(format)) {
Review Comment:
How about replacing with TableFormat#in?
##########
amoro-format-iceberg/src/main/java/org/apache/amoro/mixed/CatalogLoader.java:
##########
@@ -88,7 +88,7 @@ private static String catalogImpl(String metastoreType,
Map<String, String> cata
tableFormats.size() == 1, "Catalog support only one table format
now.");
TableFormat tableFormat = tableFormats.iterator().next();
Preconditions.checkArgument(
- TableFormat.MIXED_HIVE == tableFormat || TableFormat.MIXED_ICEBERG ==
tableFormat,
+ TableFormat.MIXED_HIVE.equals(tableFormat) ||
TableFormat.MIXED_ICEBERG.equals(tableFormat),
Review Comment:
How about replacing with TableFormat#in?
##########
amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/TableMaintainer.java:
##########
@@ -60,9 +60,9 @@ default void cleanDanglingDeleteFiles(TableRuntime
tableRuntime) {
static TableMaintainer ofTable(AmoroTable<?> amoroTable) {
TableFormat format = amoroTable.format();
- if (format == TableFormat.MIXED_HIVE || format ==
TableFormat.MIXED_ICEBERG) {
+ if (TableFormat.MIXED_HIVE.equals(format) ||
TableFormat.MIXED_ICEBERG.equals(format)) {
Review Comment:
How about replacing with TableFormat#in?
--
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]
