From dd29101efe8022999eb669876eab42acd34c0a2e Mon Sep 17 00:00:00 2001 From: baiyangtx Date: Tue, 24 Sep 2024 11:00:27 +0800 Subject: [PATCH] [Improvement]: Refactor TableFormat to a non-enumeration type to support non-open source formats (#3167) * Make TableFormat extensible * Make TableFormat extensible | fix ut * Make TableFormat extensible | fix ut * Make TableFormat extensible | fix ut * Fix json serializer && deserializer for TableFormat * Fix reviewer * Fix checkstyle * Fix checkstyle * Fix checkstyle --- .../amoro/server/AmoroServiceContainer.java | 2 + .../server/dashboard/JavalinJsonMapper.java | 99 +++++++++++++++ .../dashboard/controller/TableController.java | 22 ++-- .../maintainer/TableMaintainer.java | 4 +- .../optimizing/plan/OptimizingEvaluator.java | 4 +- .../optimizing/plan/OptimizingPlanner.java | 2 +- .../converter/TableFormatConverter.java | 72 +++++++++++ .../persistence/mapper/TableMetaMapper.java | 33 +++-- .../server/table/DefaultTableService.java | 2 +- .../amoro/server/utils/InternalTableUtil.java | 2 +- .../amoro/server/table/AMSTableTestBase.java | 20 ++- .../java/org/apache/amoro/TableFormat.java | 118 ++++++++++++++++-- .../amoro/catalog/CatalogTestHelpers.java | 4 + .../apache/amoro/utils/MixedTableUtil.java | 4 +- .../apache/amoro/catalog/TableTestBase.java | 14 +-- .../flink/catalog/FlinkUnifiedCatalog.java | 34 +++-- .../table/UnifiedDynamicTableFactory.java | 4 +- .../descriptors/MixedFormatValidator.java | 16 +-- .../amoro/spark/test/SparkTestContext.java | 4 +- .../EnableCatalogSelectExtension.java | 34 +++-- .../unified/UnifiedCatalogTestSuites.java | 16 +-- .../spark/util/MixedFormatSparkUtils.java | 10 +- .../suites/sql/TestCreateTableAsSelect.java | 2 +- .../test/suites/sql/TestCreateTableSQL.java | 4 +- .../spark/util/MixedFormatSparkUtils.java | 11 +- .../suites/sql/TestCreateTableAsSelect.java | 2 +- .../test/suites/sql/TestCreateTableSQL.java | 4 +- 27 files changed, 407 insertions(+), 136 deletions(-) create mode 100644 amoro-ams/src/main/java/org/apache/amoro/server/dashboard/JavalinJsonMapper.java create mode 100644 amoro-ams/src/main/java/org/apache/amoro/server/persistence/converter/TableFormatConverter.java diff --git a/amoro-ams/src/main/java/org/apache/amoro/server/AmoroServiceContainer.java b/amoro-ams/src/main/java/org/apache/amoro/server/AmoroServiceContainer.java index 20904ec268..146e285540 100644 --- a/amoro-ams/src/main/java/org/apache/amoro/server/AmoroServiceContainer.java +++ b/amoro-ams/src/main/java/org/apache/amoro/server/AmoroServiceContainer.java @@ -27,6 +27,7 @@ import org.apache.amoro.config.ConfigHelpers; import org.apache.amoro.config.Configurations; import org.apache.amoro.server.dashboard.DashboardServer; +import org.apache.amoro.server.dashboard.JavalinJsonMapper; import org.apache.amoro.server.dashboard.response.ErrorResponse; import org.apache.amoro.server.dashboard.utils.AmsUtil; import org.apache.amoro.server.dashboard.utils.CommonUtil; @@ -242,6 +243,7 @@ private void initHttpService() { config.addStaticFiles(dashboardServer.configStaticFiles()); config.sessionHandler(SessionHandler::new); config.enableCorsForAllOrigins(); + config.jsonMapper(JavalinJsonMapper.createDefaultJsonMapper()); config.showJavalinBanner = false; }); httpServer.routes( diff --git a/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/JavalinJsonMapper.java b/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/JavalinJsonMapper.java new file mode 100644 index 0000000000..43bda20b7f --- /dev/null +++ b/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/JavalinJsonMapper.java @@ -0,0 +1,99 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.amoro.server.dashboard; + +import io.javalin.plugin.json.JsonMapper; +import org.apache.amoro.TableFormat; +import org.apache.amoro.shade.jackson2.com.fasterxml.jackson.core.JsonProcessingException; +import org.apache.amoro.shade.jackson2.com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.amoro.shade.jackson2.com.fasterxml.jackson.databind.module.SimpleModule; +import org.jetbrains.annotations.NotNull; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; + +/** Json mapper to adapt shaded jackson. */ +public class JavalinJsonMapper implements JsonMapper { + + private final ObjectMapper objectMapper; + + public static JavalinJsonMapper createDefaultJsonMapper() { + ObjectMapper om = new ObjectMapper(); + SimpleModule module = new SimpleModule(); + module.addSerializer(TableFormat.class, new TableFormat.JsonSerializer()); + module.addDeserializer(TableFormat.class, new TableFormat.JsonDeserializer()); + om.registerModule(module); + return new JavalinJsonMapper(om); + } + + public JavalinJsonMapper(ObjectMapper shadedMapper) { + this.objectMapper = shadedMapper; + } + + @NotNull + @Override + public String toJsonString(@NotNull Object obj) { + if (obj instanceof String) { + return (String) obj; + } + try { + return objectMapper.writeValueAsString(obj); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + @NotNull + @Override + public InputStream toJsonStream(@NotNull Object obj) { + if (obj instanceof String) { + String result = (String) obj; + return new ByteArrayInputStream(result.getBytes()); + } else { + byte[] string = new byte[0]; + try { + string = objectMapper.writeValueAsBytes(obj); + return new ByteArrayInputStream(string); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + } + + @NotNull + @Override + public T fromJsonString(@NotNull String json, @NotNull Class targetClass) { + try { + return objectMapper.readValue(json, targetClass); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + @NotNull + @Override + public T fromJsonStream(@NotNull InputStream json, @NotNull Class targetClass) { + try { + return objectMapper.readValue(json, targetClass); + } catch (IOException e) { + throw new RuntimeException(e); + } + } +} diff --git a/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/TableController.java b/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/TableController.java index 754329fa7f..09544a3627 100644 --- a/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/TableController.java +++ b/amoro-ams/src/main/java/org/apache/amoro/server/dashboard/controller/TableController.java @@ -497,18 +497,16 @@ public void getTableList(Context ctx) { ServerCatalog serverCatalog = tableService.getServerCatalog(catalog); Function 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)) { + return TableMeta.TableType.ICEBERG.toString(); + } else if (format.equals(TableFormat.HUDI)) { + return TableMeta.TableType.HUDI.toString(); + } else { + return format.toString(); } }; diff --git a/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/TableMaintainer.java b/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/TableMaintainer.java index 73cd661f7c..a6ce202de5 100644 --- a/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/maintainer/TableMaintainer.java +++ b/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 (format.in(TableFormat.MIXED_HIVE, TableFormat.MIXED_ICEBERG)) { return new MixedTableMaintainer((MixedTable) amoroTable.originalTable()); - } else if (format == TableFormat.ICEBERG) { + } else if (TableFormat.ICEBERG.equals(format)) { return new IcebergTableMaintainer((Table) amoroTable.originalTable()); } else { throw new RuntimeException("Unsupported table type" + amoroTable.originalTable().getClass()); diff --git a/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/OptimizingEvaluator.java b/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/OptimizingEvaluator.java index bfd90f35da..9be5be7974 100644 --- a/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/OptimizingEvaluator.java +++ b/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/OptimizingEvaluator.java @@ -80,7 +80,7 @@ public TableRuntime getTableRuntime() { protected void initEvaluator() { long startTime = System.currentTimeMillis(); TableFileScanHelper tableFileScanHelper; - if (TableFormat.ICEBERG == mixedTable.format()) { + if (TableFormat.ICEBERG.equals(mixedTable.format())) { tableFileScanHelper = new IcebergTableFileScanHelper(mixedTable.asUnkeyedTable(), currentSnapshot.snapshotId()); } else { @@ -145,7 +145,7 @@ private Map partitionProperties(Pair partit } protected PartitionEvaluator buildEvaluator(Pair partition) { - if (TableFormat.ICEBERG == mixedTable.format()) { + if (TableFormat.ICEBERG.equals(mixedTable.format())) { return new CommonPartitionEvaluator(tableRuntime, partition, System.currentTimeMillis()); } else { Map partitionProperties = partitionProperties(partition); diff --git a/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/OptimizingPlanner.java b/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/OptimizingPlanner.java index f0c30eb43c..5985a5a342 100644 --- a/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/OptimizingPlanner.java +++ b/amoro-ams/src/main/java/org/apache/amoro/server/optimizing/plan/OptimizingPlanner.java @@ -233,7 +233,7 @@ public PartitionPlannerFactory( } public PartitionEvaluator buildPartitionPlanner(Pair partition) { - if (TableFormat.ICEBERG == mixedTable.format()) { + if (TableFormat.ICEBERG.equals(mixedTable.format())) { return new IcebergPartitionPlan(tableRuntime, mixedTable, partition, planTime); } else { if (TableTypeUtil.isHive(mixedTable)) { diff --git a/amoro-ams/src/main/java/org/apache/amoro/server/persistence/converter/TableFormatConverter.java b/amoro-ams/src/main/java/org/apache/amoro/server/persistence/converter/TableFormatConverter.java new file mode 100644 index 0000000000..ada463cd47 --- /dev/null +++ b/amoro-ams/src/main/java/org/apache/amoro/server/persistence/converter/TableFormatConverter.java @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.amoro.server.persistence.converter; + +import org.apache.amoro.TableFormat; +import org.apache.ibatis.type.JdbcType; +import org.apache.ibatis.type.MappedJdbcTypes; +import org.apache.ibatis.type.MappedTypes; +import org.apache.ibatis.type.TypeHandler; + +import java.sql.CallableStatement; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; + +@MappedTypes(TableFormat.class) +@MappedJdbcTypes(JdbcType.VARCHAR) +public class TableFormatConverter implements TypeHandler { + + @Override + public void setParameter(PreparedStatement ps, int i, TableFormat parameter, JdbcType jdbcType) + throws SQLException { + if (parameter == null) { + ps.setString(i, ""); + } else { + ps.setString(i, parameter.name()); + } + } + + @Override + public TableFormat getResult(ResultSet rs, String columnName) throws SQLException { + String res = rs.getString(columnName); + if (res == null) { + return null; + } + return TableFormat.valueOf(res); + } + + @Override + public TableFormat getResult(ResultSet rs, int columnIndex) throws SQLException { + String res = rs.getString(columnIndex); + if (res == null) { + return null; + } + return TableFormat.valueOf(res); + } + + @Override + public TableFormat getResult(CallableStatement cs, int columnIndex) throws SQLException { + String res = cs.getString(columnIndex); + if (res == null) { + return null; + } + return TableFormat.valueOf(res); + } +} diff --git a/amoro-ams/src/main/java/org/apache/amoro/server/persistence/mapper/TableMetaMapper.java b/amoro-ams/src/main/java/org/apache/amoro/server/persistence/mapper/TableMetaMapper.java index 486f7b0fbe..ead0b70066 100644 --- a/amoro-ams/src/main/java/org/apache/amoro/server/persistence/mapper/TableMetaMapper.java +++ b/amoro-ams/src/main/java/org/apache/amoro/server/persistence/mapper/TableMetaMapper.java @@ -25,6 +25,7 @@ import org.apache.amoro.server.persistence.converter.Map2StringConverter; import org.apache.amoro.server.persistence.converter.MapLong2StringConverter; import org.apache.amoro.server.persistence.converter.OptimizingStatusConverter; +import org.apache.amoro.server.persistence.converter.TableFormatConverter; import org.apache.amoro.server.table.TableMetadata; import org.apache.amoro.server.table.TableRuntime; import org.apache.ibatis.annotations.Delete; @@ -78,7 +79,10 @@ Integer decTableCount( @Result(property = "tableIdentifier.tableName", column = "table_name"), @Result(property = "tableIdentifier.database", column = "db_name"), @Result(property = "tableIdentifier.catalog", column = "catalog_name"), - @Result(property = "tableIdentifier.format", column = "format"), + @Result( + property = "tableIdentifier.format", + column = "format", + typeHandler = TableFormatConverter.class), @Result(property = "primaryKey", column = "primary_key"), @Result(property = "tableLocation", column = "table_location"), @Result(property = "baseLocation", column = "base_location"), @@ -113,7 +117,10 @@ Integer decTableCount( @Result(property = "tableIdentifier.catalog", column = "catalog_name"), @Result(property = "tableIdentifier.database", column = "db_name"), @Result(property = "tableIdentifier.tableName", column = "table_name"), - @Result(property = "tableIdentifier.format", column = "format"), + @Result( + property = "tableIdentifier.format", + column = "format", + typeHandler = TableFormatConverter.class), @Result(property = "primaryKey", column = "primary_key"), @Result(property = "tableLocation", column = "table_location"), @Result(property = "baseLocation", column = "base_location"), @@ -183,7 +190,10 @@ int commitTableChange( @Result(property = "tableIdentifier.catalog", column = "catalog_name"), @Result(property = "tableIdentifier.database", column = "db_name"), @Result(property = "tableIdentifier.tableName", column = "table_name"), - @Result(property = "tableIdentifier.format", column = "format"), + @Result( + property = "tableIdentifier.format", + column = "format", + typeHandler = TableFormatConverter.class), @Result(property = "primaryKey", column = "primary_key"), @Result(property = "tableLocation", column = "table_location"), @Result(property = "baseLocation", column = "base_location"), @@ -218,7 +228,10 @@ int commitTableChange( @Result(property = "tableIdentifier.catalog", column = "catalog_name"), @Result(property = "tableIdentifier.database", column = "db_name"), @Result(property = "tableIdentifier.tableName", column = "table_name"), - @Result(property = "tableIdentifier.format", column = "format"), + @Result( + property = "tableIdentifier.format", + column = "format", + typeHandler = TableFormatConverter.class), @Result(property = "primaryKey", column = "primary_key"), @Result(property = "tableLocation", column = "table_location"), @Result(property = "baseLocation", column = "base_location"), @@ -245,7 +258,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})") @Options(useGeneratedKeys = true, keyProperty = "tableIdentifier.id") void insertTable(@Param("tableIdentifier") ServerTableIdentifier tableIdentifier); @@ -268,7 +281,7 @@ Integer deleteTableIdByName( @Result(property = "tableName", column = "table_name"), @Result(property = "database", column = "db_name"), @Result(property = "catalog", column = "catalog_name"), - @Result(property = "format", column = "format"), + @Result(property = "format", column = "format", typeHandler = TableFormatConverter.class) }) ServerTableIdentifier selectTableIdentifier( @Param("catalogName") String catalogName, @@ -283,7 +296,7 @@ ServerTableIdentifier selectTableIdentifier( @Result(property = "catalog", column = "catalog_name"), @Result(property = "database", column = "db_name"), @Result(property = "tableName", column = "table_name"), - @Result(property = "format", column = "format") + @Result(property = "format", column = "format", typeHandler = TableFormatConverter.class) }) List selectTableIdentifiersByDb( @Param("catalogName") String catalogName, @Param("databaseName") String databaseName); @@ -296,7 +309,7 @@ List selectTableIdentifiersByDb( @Result(property = "catalog", column = "catalog_name"), @Result(property = "database", column = "db_name"), @Result(property = "tableName", column = "table_name"), - @Result(property = "format", column = "format") + @Result(property = "format", column = "format", typeHandler = TableFormatConverter.class) }) List selectTableIdentifiersByCatalog( @Param("catalogName") String catalogName); @@ -307,7 +320,7 @@ List selectTableIdentifiersByCatalog( @Result(property = "catalog", column = "catalog_name"), @Result(property = "database", column = "db_name"), @Result(property = "tableName", column = "table_name"), - @Result(property = "format", column = "format") + @Result(property = "format", column = "format", typeHandler = TableFormatConverter.class) }) List selectAllTableIdentifiers(); @@ -383,7 +396,7 @@ List selectTableIdentifiersByCatalog( @Result(property = "catalogName", column = "catalog_name"), @Result(property = "dbName", column = "db_name"), @Result(property = "tableName", column = "table_name"), - @Result(property = "format", column = "format"), + @Result(property = "format", column = "format", typeHandler = TableFormatConverter.class), @Result(property = "currentSnapshotId", column = "current_snapshot_id"), @Result(property = "currentChangeSnapshotId", column = "current_change_snapshotId"), @Result(property = "lastOptimizedSnapshotId", column = "last_optimized_snapshotId"), diff --git a/amoro-ams/src/main/java/org/apache/amoro/server/table/DefaultTableService.java b/amoro-ams/src/main/java/org/apache/amoro/server/table/DefaultTableService.java index c764d3e584..2ce56a069c 100644 --- a/amoro-ams/src/main/java/org/apache/amoro/server/table/DefaultTableService.java +++ b/amoro-ams/src/main/java/org/apache/amoro/server/table/DefaultTableService.java @@ -658,7 +658,7 @@ private boolean triggerTableAdded( AmoroTable table = catalog.loadTable( serverTableIdentifier.getDatabase(), serverTableIdentifier.getTableName()); - if (TableFormat.ICEBERG == table.format()) { + if (TableFormat.ICEBERG.equals(table.format())) { if (TablePropertyUtil.isMixedTableStore(table.properties())) { return false; } diff --git a/amoro-ams/src/main/java/org/apache/amoro/server/utils/InternalTableUtil.java b/amoro-ams/src/main/java/org/apache/amoro/server/utils/InternalTableUtil.java index 928fbce979..5d5ab39d0b 100644 --- a/amoro-ams/src/main/java/org/apache/amoro/server/utils/InternalTableUtil.java +++ b/amoro-ams/src/main/java/org/apache/amoro/server/utils/InternalTableUtil.java @@ -57,7 +57,7 @@ public class InternalTableUtil { */ public static boolean isLegacyMixedIceberg( org.apache.amoro.server.table.TableMetadata internalTableMetadata) { - return TableFormat.MIXED_ICEBERG == internalTableMetadata.getFormat() + return TableFormat.MIXED_ICEBERG.equals(internalTableMetadata.getFormat()) && !Boolean.parseBoolean( internalTableMetadata.getProperties().get(MIXED_ICEBERG_BASED_REST)); } diff --git a/amoro-ams/src/test/java/org/apache/amoro/server/table/AMSTableTestBase.java b/amoro-ams/src/test/java/org/apache/amoro/server/table/AMSTableTestBase.java index f9a8a11d20..647a2dbc87 100644 --- a/amoro-ams/src/test/java/org/apache/amoro/server/table/AMSTableTestBase.java +++ b/amoro-ams/src/test/java/org/apache/amoro/server/table/AMSTableTestBase.java @@ -174,18 +174,14 @@ protected void createTable() { TableMetadata tableMetadata = tableMetadata(); tableService().createTable(catalogMeta.getCatalogName(), tableMetadata); } else { - switch (catalogTestHelper.tableFormat()) { - case ICEBERG: - createIcebergTable(); - break; - case MIXED_ICEBERG: - createMixedIcebergTable(); - break; - case MIXED_HIVE: - createMixedHiveTable(); - break; - default: - throw new IllegalStateException("un-support format"); + if (catalogTestHelper.tableFormat().equals(TableFormat.ICEBERG)) { + createIcebergTable(); + } else if (catalogTestHelper.tableFormat().equals(TableFormat.MIXED_ICEBERG)) { + createMixedIcebergTable(); + } else if (catalogTestHelper.tableFormat().equals(TableFormat.MIXED_HIVE)) { + createMixedHiveTable(); + } else { + throw new IllegalStateException("un-support format"); } tableService().exploreExternalCatalog(); } diff --git a/amoro-common/src/main/java/org/apache/amoro/TableFormat.java b/amoro-common/src/main/java/org/apache/amoro/TableFormat.java index 31b94b146a..62400267a8 100644 --- a/amoro-common/src/main/java/org/apache/amoro/TableFormat.java +++ b/amoro-common/src/main/java/org/apache/amoro/TableFormat.java @@ -18,24 +18,128 @@ package org.apache.amoro; +import org.apache.amoro.shade.guava32.com.google.common.base.Preconditions; +import org.apache.amoro.shade.guava32.com.google.common.collect.Maps; +import org.apache.amoro.shade.jackson2.com.fasterxml.jackson.core.JsonGenerator; +import org.apache.amoro.shade.jackson2.com.fasterxml.jackson.core.JsonParser; +import org.apache.amoro.shade.jackson2.com.fasterxml.jackson.core.JsonProcessingException; +import org.apache.amoro.shade.jackson2.com.fasterxml.jackson.core.TreeNode; +import org.apache.amoro.shade.jackson2.com.fasterxml.jackson.databind.DeserializationContext; +import org.apache.amoro.shade.jackson2.com.fasterxml.jackson.databind.SerializerProvider; + +import java.io.IOException; +import java.io.Serializable; +import java.util.Map; + /** * Table formats Amoro supported * * @since 0.4.0 */ -public enum TableFormat { - ICEBERG, - MIXED_ICEBERG, - MIXED_HIVE, - PAIMON, - HUDI; +public final class TableFormat implements Serializable { + private static final Map registeredFormats = Maps.newConcurrentMap(); + + /** Open-source table formats */ + public static final TableFormat ICEBERG = register("ICEBERG"); + + public static final TableFormat MIXED_ICEBERG = register("MIXED_ICEBERG"); + public static final TableFormat MIXED_HIVE = register("MIXED_HIVE"); + public static final TableFormat PAIMON = register("PAIMON"); + public static final TableFormat HUDI = register("HUDI"); + + /** + * Get all registered formats + * + * @return registered formats + */ + public static TableFormat[] values() { + return registeredFormats.values().toArray(new TableFormat[0]); + } + + /** + * Register a new TableFormat + * + * @param name table format name + * @return TableFormat. + */ + public static TableFormat register(String name) { + return registeredFormats.computeIfAbsent(name, s -> new TableFormat(name)); + } + + /** + * Get TableFormat by name + * + * @param name name + * @return TableFormat + */ + public static TableFormat valueOf(String name) { + return registeredFormats.get(name); + } + + private final String name; + + private TableFormat(String name) { + Preconditions.checkNotNull(name, "TableFormat name should not be null"); + this.name = name; + } + + public String name() { + return name; + } public boolean in(TableFormat... tableFormats) { for (TableFormat tableFormat : tableFormats) { - if (this == tableFormat) { + if (this.equals(tableFormat)) { return true; } } return false; } + + @Override + public String toString() { + return this.name; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } else if (other == null || getClass() != other.getClass()) { + return false; + } + return this.name.equals(((TableFormat) other).name); + } + + @Override + public int hashCode() { + return this.name.hashCode(); + } + + /** Json deserializer for TableFormat */ + public static class JsonDeserializer + extends org.apache.amoro.shade.jackson2.com.fasterxml.jackson.databind.JsonDeserializer< + TableFormat> { + + @Override + public TableFormat deserialize( + JsonParser jsonParser, DeserializationContext deserializationContext) + throws IOException, JsonProcessingException { + TreeNode node = jsonParser.getCodec().readTree(jsonParser); + return TableFormat.valueOf(node.toString()); + } + } + + /** Json serializer for TableFormat */ + public static class JsonSerializer + extends org.apache.amoro.shade.jackson2.com.fasterxml.jackson.databind.JsonSerializer< + TableFormat> { + + @Override + public void serialize( + TableFormat tableFormat, JsonGenerator jsonGenerator, SerializerProvider serializerProvider) + throws IOException { + jsonGenerator.writeString(tableFormat.name()); + } + } } diff --git a/amoro-common/src/test/java/org/apache/amoro/catalog/CatalogTestHelpers.java b/amoro-common/src/test/java/org/apache/amoro/catalog/CatalogTestHelpers.java index 18d8dec5f3..e0b6cbb813 100644 --- a/amoro-common/src/test/java/org/apache/amoro/catalog/CatalogTestHelpers.java +++ b/amoro-common/src/test/java/org/apache/amoro/catalog/CatalogTestHelpers.java @@ -51,6 +51,10 @@ public static CatalogMeta buildCatalogMeta( CatalogMetaProperties.STORAGE_CONFIGS_KEY_CORE_SITE, HADOOP_EMPTY_CONFIG_BASE64); storageConfig.put( CatalogMetaProperties.STORAGE_CONFIGS_KEY_HDFS_SITE, HADOOP_EMPTY_CONFIG_BASE64); + if (CatalogMetaProperties.CATALOG_TYPE_HIVE.equalsIgnoreCase(type)) { + storageConfig.put( + CatalogMetaProperties.STORAGE_CONFIGS_KEY_HIVE_SITE, HADOOP_EMPTY_CONFIG_BASE64); + } Map authConfig = new HashMap<>(); authConfig.put( diff --git a/amoro-format-iceberg/src/main/java/org/apache/amoro/utils/MixedTableUtil.java b/amoro-format-iceberg/src/main/java/org/apache/amoro/utils/MixedTableUtil.java index e94998c78b..19947678bf 100644 --- a/amoro-format-iceberg/src/main/java/org/apache/amoro/utils/MixedTableUtil.java +++ b/amoro-format-iceberg/src/main/java/org/apache/amoro/utils/MixedTableUtil.java @@ -53,7 +53,7 @@ public static UnkeyedTable baseStore(MixedTable mixedTable) { /** Return the table root location of the mixed-format table. */ public static String tableRootLocation(MixedTable mixedTable) { String tableRootLocation; - if (TableFormat.ICEBERG != mixedTable.format() && mixedTable.isUnkeyedTable()) { + if (!TableFormat.ICEBERG.equals(mixedTable.format()) && mixedTable.isUnkeyedTable()) { tableRootLocation = TableFileUtil.getFileDir(mixedTable.location()); } else { tableRootLocation = mixedTable.location(); @@ -179,7 +179,7 @@ private static StructLikeMap readLegacyPartitionProperties( * Mix format table will return directly after checking}. */ public static PartitionSpec getMixedTablePartitionSpecById(MixedTable mixedTable, int specId) { - if (mixedTable.format() == TableFormat.ICEBERG) { + if (TableFormat.ICEBERG.equals(mixedTable.format())) { return mixedTable.asUnkeyedTable().specs().get(specId); } else { PartitionSpec spec = mixedTable.spec(); diff --git a/amoro-format-iceberg/src/test/java/org/apache/amoro/catalog/TableTestBase.java b/amoro-format-iceberg/src/test/java/org/apache/amoro/catalog/TableTestBase.java index 7e64192562..ae433b2089 100644 --- a/amoro-format-iceberg/src/test/java/org/apache/amoro/catalog/TableTestBase.java +++ b/amoro-format-iceberg/src/test/java/org/apache/amoro/catalog/TableTestBase.java @@ -18,6 +18,7 @@ package org.apache.amoro.catalog; +import org.apache.amoro.TableFormat; import org.apache.amoro.TableTestHelper; import org.apache.amoro.table.MixedTable; import org.apache.amoro.table.TableBuilder; @@ -44,14 +45,11 @@ public void setupTable() { this.tableMetaStore = CatalogUtil.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 (format.in(TableFormat.MIXED_HIVE, TableFormat.MIXED_ICEBERG)) { + createMixedFormatTable(); + } else if (TableFormat.ICEBERG.equals(format)) { + createIcebergFormatTable(); } } diff --git a/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/catalog/FlinkUnifiedCatalog.java b/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/catalog/FlinkUnifiedCatalog.java index 3d944adb72..2c5c61f7c7 100644 --- a/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/catalog/FlinkUnifiedCatalog.java +++ b/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/catalog/FlinkUnifiedCatalog.java @@ -220,7 +220,7 @@ public void createTable(ObjectPath tablePath, CatalogBaseTable table, boolean ig throws TableAlreadyExistException, DatabaseNotExistException, CatalogException { Configuration configuration = new Configuration(); table.getOptions().forEach(configuration::setString); - TableFormat format = configuration.get(TABLE_FORMAT); + TableFormat format = TableFormat.valueOf(configuration.get(TABLE_FORMAT)); TableIdentifier tableIdentifier = TableIdentifier.of( unifiedCatalog.name(), tablePath.getDatabaseName(), tablePath.getObjectName()); @@ -461,25 +461,19 @@ private AmoroTable loadAmoroTable(ObjectPath tablePath) { private AbstractCatalog createOriginalCatalog( TableIdentifier tableIdentifier, TableFormat tableFormat) { CatalogFactory catalogFactory; - - switch (tableFormat) { - case MIXED_ICEBERG: - case MIXED_HIVE: - catalogFactory = new MixedCatalogFactory(); - break; - case ICEBERG: - catalogFactory = new IcebergFlinkCatalogFactory(hadoopConf); - break; - case PAIMON: - catalogFactory = - new PaimonFlinkCatalogFactory( - unifiedCatalog.properties(), unifiedCatalog.metastoreType()); - break; - default: - throw new UnsupportedOperationException( - String.format( - "Unsupported table format: [%s] in the unified catalog, table identifier is [%s], the supported table formats are [%s].", - tableFormat, tableIdentifier, FlinkUnifiedCatalogFactory.SUPPORTED_FORMATS)); + if (tableFormat.in(TableFormat.MIXED_HIVE, TableFormat.MIXED_ICEBERG)) { + catalogFactory = new MixedCatalogFactory(); + } else if (tableFormat.equals(TableFormat.ICEBERG)) { + catalogFactory = new IcebergFlinkCatalogFactory(hadoopConf); + } else if (tableFormat.equals(TableFormat.PAIMON)) { + catalogFactory = + new PaimonFlinkCatalogFactory( + unifiedCatalog.properties(), unifiedCatalog.metastoreType()); + } else { + throw new UnsupportedOperationException( + String.format( + "Unsupported table format: [%s] in the unified catalog, table identifier is [%s], the supported table formats are [%s].", + tableFormat, tableIdentifier, FlinkUnifiedCatalogFactory.SUPPORTED_FORMATS)); } AbstractCatalog originalCatalog; diff --git a/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/table/UnifiedDynamicTableFactory.java b/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/table/UnifiedDynamicTableFactory.java index 203a823f76..71f30995f9 100644 --- a/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/table/UnifiedDynamicTableFactory.java +++ b/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/table/UnifiedDynamicTableFactory.java @@ -57,7 +57,7 @@ public DynamicTableSink createDynamicTableSink(Context context) { ObjectIdentifier identifier = context.getObjectIdentifier(); FactoryUtil.TableFactoryHelper helper = FactoryUtil.createTableFactoryHelper(this, context); Configuration options = (Configuration) helper.getOptions(); - TableFormat tableFormat = options.get(MixedFormatValidator.TABLE_FORMAT); + TableFormat tableFormat = TableFormat.valueOf(options.get(MixedFormatValidator.TABLE_FORMAT)); return getOriginalCatalog(tableFormat) .flatMap(AbstractCatalog::getFactory) @@ -76,7 +76,7 @@ public DynamicTableSource createDynamicTableSource(Context context) { ObjectIdentifier identifier = context.getObjectIdentifier(); FactoryUtil.TableFactoryHelper helper = FactoryUtil.createTableFactoryHelper(this, context); Configuration options = (Configuration) helper.getOptions(); - TableFormat tableFormat = options.get(MixedFormatValidator.TABLE_FORMAT); + TableFormat tableFormat = TableFormat.valueOf(options.get(MixedFormatValidator.TABLE_FORMAT)); return getOriginalCatalog(tableFormat) .flatMap(AbstractCatalog::getFactory) diff --git a/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/table/descriptors/MixedFormatValidator.java b/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/table/descriptors/MixedFormatValidator.java index 51831749e9..662da35be3 100644 --- a/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/table/descriptors/MixedFormatValidator.java +++ b/amoro-format-mixed/amoro-mixed-flink/amoro-mixed-flink-common/src/main/java/org/apache/amoro/flink/table/descriptors/MixedFormatValidator.java @@ -283,18 +283,18 @@ public class MixedFormatValidator extends ConnectorDescriptorValidator { + " of the key. Default is -1, means it is automatically determined: every shard will be at least 512KB and" + " number of shard bits will not exceed 6."); - public static final ConfigOption TABLE_FORMAT = + public static final ConfigOption TABLE_FORMAT = ConfigOptions.key("table.format") - .enumType(TableFormat.class) - .defaultValue(TableFormat.MIXED_ICEBERG) + .stringType() + .defaultValue(TableFormat.MIXED_ICEBERG.name()) .withDescription( String.format( "The format of the table, valid values are %s, %s, %s or %s, and Flink choose '%s' as default format.", - TableFormat.ICEBERG, - TableFormat.MIXED_ICEBERG, - TableFormat.MIXED_HIVE, - TableFormat.PAIMON, - TableFormat.MIXED_ICEBERG)); + TableFormat.ICEBERG.name(), + TableFormat.MIXED_ICEBERG.name(), + TableFormat.MIXED_HIVE.name(), + TableFormat.PAIMON.name(), + TableFormat.MIXED_ICEBERG.name())); public static final ConfigOption SCAN_PARALLELISM = ConfigOptions.key("source.parallelism") diff --git a/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/SparkTestContext.java b/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/SparkTestContext.java index 277225eaa4..351264ecef 100644 --- a/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/SparkTestContext.java +++ b/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/SparkTestContext.java @@ -121,7 +121,7 @@ private void setupCatalogs() { } HiveConf hiveConf = hms.getHiveConf(); for (TableFormat format : TableFormat.values()) { - if (format == TableFormat.HUDI) { + if (TableFormat.HUDI.equals(format)) { continue; } // create catalog for all formats in AMS with hive metastore. @@ -140,7 +140,7 @@ private void setupCatalogs() { Joiner.on(',') .join( Arrays.stream(TableFormat.values()) - .filter(f -> TableFormat.HUDI != f) + .filter(f -> !TableFormat.HUDI.equals(f)) .collect(Collectors.toList())); allFormats.putToCatalogProperties(CatalogMetaProperties.TABLE_FORMATS, formats); allFormats.setCatalogName(AMS_ALL_FORMAT_CATALOG_NAME); diff --git a/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/extensions/EnableCatalogSelectExtension.java b/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/extensions/EnableCatalogSelectExtension.java index b518e2479f..021d770be2 100644 --- a/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/extensions/EnableCatalogSelectExtension.java +++ b/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/extensions/EnableCatalogSelectExtension.java @@ -93,30 +93,28 @@ private String selectMixedCatalogByFormat(ExtensionContext context, ExtensionReg Preconditions.condition( format == TableFormat.MIXED_ICEBERG || format == TableFormat.MIXED_HIVE, "must be a mixed-format"); - switch (format) { - case MIXED_ICEBERG: - return SparkTestContext.SparkCatalogNames.MIXED_ICEBERG; - case MIXED_HIVE: - return SparkTestContext.SparkCatalogNames.MIXED_HIVE; - default: - throw new IllegalArgumentException("must be a mixed-format"); + if (TableFormat.MIXED_HIVE.equals(format)) { + return SparkTestContext.SparkCatalogNames.MIXED_HIVE; + } else if (TableFormat.MIXED_ICEBERG.equals(format)) { + return SparkTestContext.SparkCatalogNames.MIXED_ICEBERG; + } else { + throw new IllegalArgumentException("must be a mixed-format"); } } private String selectUnifiedCatalogByFormat( ExtensionContext context, ExtensionRegistry registry) { TableFormat format = formatFromMethodArgs(context, registry); - switch (format) { - case MIXED_ICEBERG: - return SparkTestContext.SparkCatalogNames.UNIFIED_MIXED_ICEBERG; - case MIXED_HIVE: - return SparkTestContext.SparkCatalogNames.UNIFIED_MIXED_HIVE; - case ICEBERG: - return SparkTestContext.SparkCatalogNames.UNIFIED_ICEBERG; - case PAIMON: - return SparkTestContext.SparkCatalogNames.UNIFIED_PAIMON; - default: - throw new IllegalArgumentException("unknown format"); + if (TableFormat.MIXED_HIVE.equals(format)) { + return SparkTestContext.SparkCatalogNames.UNIFIED_MIXED_HIVE; + } else if (TableFormat.MIXED_ICEBERG.equals(format)) { + return SparkTestContext.SparkCatalogNames.UNIFIED_MIXED_ICEBERG; + } else if (TableFormat.ICEBERG.equals(format)) { + return SparkTestContext.SparkCatalogNames.UNIFIED_ICEBERG; + } else if (TableFormat.PAIMON.equals(format)) { + return SparkTestContext.SparkCatalogNames.UNIFIED_PAIMON; + } else { + throw new IllegalArgumentException("must be a mixed-format"); } } diff --git a/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/unified/UnifiedCatalogTestSuites.java b/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/unified/UnifiedCatalogTestSuites.java index b356842977..480cd54e19 100644 --- a/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/unified/UnifiedCatalogTestSuites.java +++ b/amoro-format-mixed/amoro-mixed-spark/amoro-mixed-spark-3-common/src/test/java/org/apache/amoro/spark/test/unified/UnifiedCatalogTestSuites.java @@ -76,7 +76,7 @@ public void testTableFormats(TableFormat format, boolean sessionCatalog) { + " PARTITIONED BY (pt) "; sql(sqlText); int expect = 0; - if (TableFormat.PAIMON != format || !spark().version().startsWith("3.1")) { + if (!TableFormat.PAIMON.equals(format) || !spark().version().startsWith("3.1")) { // write is not supported in spark3-1 sqlText = "INSERT INTO " @@ -109,7 +109,7 @@ public void testTableFormats(TableFormat format, boolean sessionCatalog) { } private String pkDDL(TableFormat format) { - if (TableFormat.MIXED_HIVE == format || TableFormat.MIXED_ICEBERG == format) { + if (TableFormat.MIXED_HIVE.equals(format) || TableFormat.MIXED_ICEBERG.equals(format)) { return ", primary key(id)"; } return ""; @@ -147,14 +147,10 @@ private void testVisitSubTable(TableFormat format, boolean sessionCatalog) { } List subTableNames = Lists.newArrayList(); - switch (format) { - case ICEBERG: - subTableNames = icebergInspectTableNames(); - break; - case MIXED_ICEBERG: - case MIXED_HIVE: - subTableNames = mixedFormatSubTableNames(); - break; + if (TableFormat.ICEBERG.equals(format)) { + subTableNames = icebergInspectTableNames(); + } else if (format.in(TableFormat.MIXED_HIVE, TableFormat.MIXED_ICEBERG)) { + subTableNames = mixedFormatSubTableNames(); } for (String inspectTableName : subTableNames) { diff --git a/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/main/java/org/apache/amoro/spark/util/MixedFormatSparkUtils.java b/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/main/java/org/apache/amoro/spark/util/MixedFormatSparkUtils.java index 6ef07a002c..52a4b90b3b 100644 --- a/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/main/java/org/apache/amoro/spark/util/MixedFormatSparkUtils.java +++ b/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/main/java/org/apache/amoro/spark/util/MixedFormatSparkUtils.java @@ -22,6 +22,7 @@ import static org.apache.amoro.table.TableProperties.WRITE_DISTRIBUTION_MODE_DEFAULT; import static org.apache.iceberg.spark.Spark3Util.toTransforms; +import org.apache.amoro.TableFormat; import org.apache.amoro.shade.guava32.com.google.common.base.Joiner; import org.apache.amoro.shade.guava32.com.google.common.base.Preconditions; import org.apache.amoro.spark.table.MixedSparkTable; @@ -186,12 +187,9 @@ public static Object convertConstant(Type type, Object value) { } public static String mixedTableProvider(MixedTable table) { - switch (table.format()) { - case MIXED_ICEBERG: - case MIXED_HIVE: - return table.format().name().toLowerCase(Locale.ROOT); - default: - throw new IllegalArgumentException("Not a mixed-format table:" + table.format()); + if (table.format().in(TableFormat.MIXED_ICEBERG, TableFormat.MIXED_HIVE)) { + return table.format().name().toLowerCase(Locale.ROOT); } + throw new IllegalArgumentException("Not a mixed-format table:" + table.format()); } } diff --git a/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableAsSelect.java b/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableAsSelect.java index 8953f3267d..04b61a76f3 100644 --- a/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableAsSelect.java +++ b/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableAsSelect.java @@ -233,7 +233,7 @@ public void testSchemaAndData( TableFiles files = TestTableUtil.files(table); Asserts.assertAllFilesInBaseStore(files); - if (TableFormat.MIXED_HIVE == format) { + if (TableFormat.MIXED_HIVE.equals(format)) { Table hiveTable = loadHiveTable(); Asserts.assertHiveColumns(expectSchema, ptSpec, hiveTable.getSd().getCols()); Asserts.assertHivePartition(ptSpec, hiveTable.getPartitionKeys()); diff --git a/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java b/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java index eb75451215..99af2f5d7e 100644 --- a/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java +++ b/amoro-format-mixed/amoro-mixed-spark/v3.2/amoro-mixed-spark-3.2/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java @@ -218,7 +218,7 @@ public void testPartitionSpec(TableFormat format, String partitionDDL, Partition MixedTable actualTable = loadTable(); Asserts.assertPartition(expectSpec, actualTable.spec()); - if (TableFormat.MIXED_HIVE == format) { + if (TableFormat.MIXED_HIVE.equals(format)) { Table hiveTable = loadHiveTable(); Asserts.assertHivePartition(expectSpec, hiveTable.getPartitionKeys()); } @@ -303,7 +303,7 @@ public void testSchemaAndProperties( Asserts.assertType(expectSchema.asStruct(), tbl.schema().asStruct()); Asserts.assertHashMapContainExpect(expectProperties, tbl.properties()); - if (TableFormat.MIXED_HIVE == format) { + if (TableFormat.MIXED_HIVE.equals(format)) { Table hiveTable = loadHiveTable(); Asserts.assertHiveColumns( expectSchema, PartitionSpec.unpartitioned(), hiveTable.getSd().getCols()); diff --git a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/main/java/org/apache/amoro/spark/util/MixedFormatSparkUtils.java b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/main/java/org/apache/amoro/spark/util/MixedFormatSparkUtils.java index 6ef07a002c..925a56c82c 100644 --- a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/main/java/org/apache/amoro/spark/util/MixedFormatSparkUtils.java +++ b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/main/java/org/apache/amoro/spark/util/MixedFormatSparkUtils.java @@ -22,6 +22,7 @@ import static org.apache.amoro.table.TableProperties.WRITE_DISTRIBUTION_MODE_DEFAULT; import static org.apache.iceberg.spark.Spark3Util.toTransforms; +import org.apache.amoro.TableFormat; import org.apache.amoro.shade.guava32.com.google.common.base.Joiner; import org.apache.amoro.shade.guava32.com.google.common.base.Preconditions; import org.apache.amoro.spark.table.MixedSparkTable; @@ -186,12 +187,10 @@ public static Object convertConstant(Type type, Object value) { } public static String mixedTableProvider(MixedTable table) { - switch (table.format()) { - case MIXED_ICEBERG: - case MIXED_HIVE: - return table.format().name().toLowerCase(Locale.ROOT); - default: - throw new IllegalArgumentException("Not a mixed-format table:" + table.format()); + if (table.format().in(TableFormat.MIXED_HIVE, TableFormat.MIXED_ICEBERG)) { + return table.format().name().toLowerCase(Locale.ROOT); + } else { + throw new IllegalArgumentException("Not a mixed-format table:" + table.format()); } } } diff --git a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableAsSelect.java b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableAsSelect.java index aeb0de686d..bdc773e9f1 100644 --- a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableAsSelect.java +++ b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableAsSelect.java @@ -234,7 +234,7 @@ public void testSchemaAndData( TableFiles files = TestTableUtil.files(table); Asserts.assertAllFilesInBaseStore(files); - if (TableFormat.MIXED_HIVE == format) { + if (TableFormat.MIXED_HIVE.equals(format)) { Table hiveTable = loadHiveTable(); Asserts.assertHiveColumns(expectSchema, ptSpec, hiveTable.getSd().getCols()); Asserts.assertHivePartition(ptSpec, hiveTable.getPartitionKeys()); diff --git a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java index f13f645c9a..6142def5aa 100644 --- a/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java +++ b/amoro-format-mixed/amoro-mixed-spark/v3.3/amoro-mixed-spark-3.3/src/test/java/org/apache/amoro/spark/test/suites/sql/TestCreateTableSQL.java @@ -218,7 +218,7 @@ public void testPartitionSpec(TableFormat format, String partitionDDL, Partition MixedTable actualTable = loadTable(); Asserts.assertPartition(expectSpec, actualTable.spec()); - if (TableFormat.MIXED_HIVE == format) { + if (TableFormat.MIXED_HIVE.equals(format)) { Table hiveTable = loadHiveTable(); Asserts.assertHivePartition(expectSpec, hiveTable.getPartitionKeys()); } @@ -303,7 +303,7 @@ public void testSchemaAndProperties( Asserts.assertType(expectSchema.asStruct(), tbl.schema().asStruct()); Asserts.assertHashMapContainExpect(expectProperties, tbl.properties()); - if (TableFormat.MIXED_HIVE == format) { + if (TableFormat.MIXED_HIVE.equals(format)) { Table hiveTable = loadHiveTable(); Asserts.assertHiveColumns( expectSchema, PartitionSpec.unpartitioned(), hiveTable.getSd().getCols());