From 92ab88e702c0a7e1cd974c08f7c9ccbf84b81765 Mon Sep 17 00:00:00 2001 From: caosurong Date: Tue, 1 Sep 2026 15:52:48 +0800 Subject: [PATCH 1/3] Fix Hive view filtering in table management --- .../dashboard/controller/TableController.java | 10 +- .../java/org/apache/amoro/hive/HMSClient.java | 15 ++ .../org/apache/amoro/hive/HMSClientImpl.java | 7 + .../apache/amoro/hive/HiveTableTypeUtil.java | 112 +++++++++++++ .../java/org/apache/amoro/hive/TestHMS.java | 36 +++++ .../amoro/hive/TestHiveTableTypeUtil.java | 46 ++++++ .../amoro/formats/iceberg/IcebergCatalog.java | 60 ++++++- .../iceberg/IcebergCatalogFactory.java | 2 +- .../amoro/hive/catalog/MixedHiveCatalog.java | 20 ++- .../amoro/hive/utils/HiveTableUtil.java | 39 ++++- .../hive/catalog/TestMixedHiveCatalog.java | 22 +++ .../formats/IcebergHiveCatalogTestHelper.java | 5 +- .../formats/TestIcebergHiveAmoroCatalog.java | 35 +++++ .../amoro/hive/utils/TestHiveTableUtil.java | 147 ++++++++++++++++++ 14 files changed, 542 insertions(+), 14 deletions(-) create mode 100644 amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java create mode 100644 amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java create mode 100644 amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java 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 772e8d0ddd..087acdbc55 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 @@ -199,7 +199,12 @@ public void getHiveTableDetail(Context ctx) { TableIdentifier tableIdentifier = TableIdentifier.of(catalog, db, table); HiveTableInfo hiveTableInfo; - Table hiveTable = HiveTableUtil.loadHmsTable(hmsClientPool, tableIdentifier); + Table hiveTable = HiveTableUtil.loadPhysicalHmsTable(hmsClientPool, tableIdentifier); + Preconditions.checkState( + hiveTable.getSd() != null, + "Hive table %s.%s does not have a storage descriptor", + db, + table); List schema = transformHiveSchemaToAMSColumnInfo(hiveTable.getSd().getCols()); List partitionColumnInfos = transformHiveSchemaToAMSColumnInfo(hiveTable.getPartitionKeys()); @@ -727,6 +732,9 @@ private void putMainBranchFirst(List branchInfos) { } private List transformHiveSchemaToAMSColumnInfo(List fields) { + if (fields == null) { + return Collections.emptyList(); + } return fields.stream() .map( f -> { diff --git a/amoro-common/src/main/java/org/apache/amoro/hive/HMSClient.java b/amoro-common/src/main/java/org/apache/amoro/hive/HMSClient.java index 140324abc4..f4c7751772 100644 --- a/amoro-common/src/main/java/org/apache/amoro/hive/HMSClient.java +++ b/amoro-common/src/main/java/org/apache/amoro/hive/HMSClient.java @@ -24,6 +24,7 @@ import org.apache.hadoop.hive.metastore.api.MetaException; import org.apache.hadoop.hive.metastore.api.Partition; import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.TableMeta; import org.apache.thrift.TException; import java.lang.reflect.InvocationTargetException; @@ -88,4 +89,18 @@ void alterPartitions( InvocationTargetException, ClassNotFoundException; List getTableObjectsByName(String dbName, List tableNames) throws TException; + + /** + * Returns lightweight table metadata matching the database, table, and table type patterns. + * + * @param databasePattern Hive database name or pattern + * @param tablePattern Hive table name or pattern + * @param tableTypes Hive table types to include + * @return matching lightweight table metadata + * @throws UnsupportedOperationException if the client implementation does not support this API + */ + default List getTableMeta( + String databasePattern, String tablePattern, List tableTypes) throws TException { + throw new UnsupportedOperationException("Getting Hive table metadata is not supported"); + } } diff --git a/amoro-common/src/main/java/org/apache/amoro/hive/HMSClientImpl.java b/amoro-common/src/main/java/org/apache/amoro/hive/HMSClientImpl.java index 85ddb6833e..a1d49d0148 100644 --- a/amoro-common/src/main/java/org/apache/amoro/hive/HMSClientImpl.java +++ b/amoro-common/src/main/java/org/apache/amoro/hive/HMSClientImpl.java @@ -26,6 +26,7 @@ import org.apache.hadoop.hive.metastore.api.MetaException; import org.apache.hadoop.hive.metastore.api.Partition; import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.TableMeta; import org.apache.thrift.TException; import java.util.List; @@ -187,4 +188,10 @@ public List
getTableObjectsByName(String dbName, List tableNames) throws TException { return getClient().getTableObjectsByName(dbName, tableNames); } + + @Override + public List getTableMeta( + String databasePattern, String tablePattern, List tableTypes) throws TException { + return getClient().getTableMeta(databasePattern, tablePattern, tableTypes); + } } diff --git a/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java b/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java new file mode 100644 index 0000000000..be9bfa4ba0 --- /dev/null +++ b/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java @@ -0,0 +1,112 @@ +/* + * 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.hive; + +import org.apache.hadoop.hive.metastore.TableType; +import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.TableMeta; +import org.apache.thrift.TException; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import java.util.stream.Collectors; + +/** Utilities for classifying Hive Metastore table objects. */ +public final class HiveTableTypeUtil { + + private static final List VIEW_TYPES = + Collections.unmodifiableList( + Arrays.asList(TableType.VIRTUAL_VIEW.name(), TableType.MATERIALIZED_VIEW.name())); + + private HiveTableTypeUtil() {} + + /** Returns the Hive table type names representing virtual or materialized views. */ + public static List viewTypes() { + return VIEW_TYPES; + } + + /** Returns whether the Hive table is a virtual or materialized view. */ + public static boolean isView(Table table) { + return table != null && isViewType(table.getTableType()); + } + + /** Returns whether the lightweight Hive table metadata describes a view. */ + public static boolean isView(TableMeta table) { + return table != null && isViewType(table.getTableType()); + } + + /** + * Returns the normalized names of Hive views among the candidate tables. + * + *

The lightweight table metadata API is preferred. Clients that do not implement that API fall + * back to loading the candidate table objects. + * + * @param client Hive Metastore client + * @param database database containing the candidate tables + * @param candidateTableNames table names to inspect + * @return lowercase names of virtual and materialized views + */ + public static Set listViewNames( + HMSClient client, String database, List candidateTableNames) throws TException { + if (candidateTableNames == null || candidateTableNames.isEmpty()) { + return Collections.emptySet(); + } + + List views; + try { + views = client.getTableMeta(database, "*", viewTypes()); + } catch (UnsupportedOperationException e) { + return listViewNamesFromTables(client, database, candidateTableNames); + } + + if (views == null) { + return listViewNamesFromTables(client, database, candidateTableNames); + } + return views.stream() + .filter(HiveTableTypeUtil::isView) + .map(TableMeta::getTableName) + .filter(name -> name != null) + .map(name -> name.toLowerCase(Locale.ROOT)) + .collect(Collectors.toCollection(HashSet::new)); + } + + private static Set listViewNamesFromTables( + HMSClient client, String database, List candidateTableNames) throws TException { + List

tables = client.getTableObjectsByName(database, candidateTableNames); + if (tables == null) { + throw new IllegalStateException( + "Hive Metastore returned null while loading table objects from database: " + database); + } + return tables.stream() + .filter(HiveTableTypeUtil::isView) + .map(Table::getTableName) + .filter(name -> name != null) + .map(name -> name.toLowerCase(Locale.ROOT)) + .collect(Collectors.toCollection(HashSet::new)); + } + + private static boolean isViewType(String tableType) { + return TableType.VIRTUAL_VIEW.name().equalsIgnoreCase(tableType) + || TableType.MATERIALIZED_VIEW.name().equalsIgnoreCase(tableType); + } +} diff --git a/amoro-common/src/test/java/org/apache/amoro/hive/TestHMS.java b/amoro-common/src/test/java/org/apache/amoro/hive/TestHMS.java index b7f2efaf80..b61e5f9a58 100644 --- a/amoro-common/src/test/java/org/apache/amoro/hive/TestHMS.java +++ b/amoro-common/src/test/java/org/apache/amoro/hive/TestHMS.java @@ -21,6 +21,12 @@ import org.apache.amoro.SingletonResourceUtil; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.HiveMetaStoreClient; +import org.apache.hadoop.hive.metastore.TableType; +import org.apache.hadoop.hive.metastore.api.FieldSchema; +import org.apache.hadoop.hive.metastore.api.SerDeInfo; +import org.apache.hadoop.hive.metastore.api.StorageDescriptor; +import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.thrift.TException; import org.junit.rules.ExternalResource; import org.junit.rules.TemporaryFolder; import org.slf4j.Logger; @@ -28,6 +34,9 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; public class TestHMS extends ExternalResource { private static final Logger LOG = LoggerFactory.getLogger(TestHMS.class); @@ -71,6 +80,33 @@ public HiveMetaStoreClient getHiveClient() { return mockHms.getClient(); } + public void createView(String database, String viewName) throws TException { + createView(database, viewName, Collections.emptyMap()); + } + + public void createView(String database, String viewName, Map parameters) + throws TException { + StorageDescriptor storageDescriptor = new StorageDescriptor(); + storageDescriptor.setCols( + Collections.singletonList(new FieldSchema("id", "int", "view column"))); + storageDescriptor.setSerdeInfo(new SerDeInfo()); + Table view = + new Table( + viewName, + database, + System.getProperty("user.name"), + (int) (System.currentTimeMillis() / 1000), + 0, + 0, + storageDescriptor, + Collections.emptyList(), + new HashMap<>(parameters), + "select 1 as id", + "select 1 as id", + TableType.VIRTUAL_VIEW.name()); + getHiveClient().createTable(view); + } + public int getMetastorePort() { return mockHms.getMetastorePort(); } diff --git a/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java b/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java new file mode 100644 index 0000000000..e7badc7b69 --- /dev/null +++ b/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java @@ -0,0 +1,46 @@ +/* + * 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.hive; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.hadoop.hive.metastore.TableType; +import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.TableMeta; +import org.junit.jupiter.api.Test; + +public class TestHiveTableTypeUtil { + + @Test + public void testViewTypes() { + assertTrue(HiveTableTypeUtil.isView(tableWithType(TableType.VIRTUAL_VIEW.name()))); + assertTrue(HiveTableTypeUtil.isView(tableWithType(TableType.MATERIALIZED_VIEW.name()))); + assertFalse(HiveTableTypeUtil.isView(tableWithType(TableType.EXTERNAL_TABLE.name()))); + assertFalse(HiveTableTypeUtil.isView(tableWithType(null))); + assertTrue( + HiveTableTypeUtil.isView(new TableMeta("database", "view", TableType.VIRTUAL_VIEW.name()))); + } + + private static Table tableWithType(String tableType) { + Table table = new Table(); + table.setTableType(tableType); + return table; + } +} diff --git a/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalog.java b/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalog.java index fbe1ff3731..9125eec7fb 100644 --- a/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalog.java +++ b/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalog.java @@ -18,8 +18,14 @@ package org.apache.amoro.formats.iceberg; +import static org.apache.iceberg.CatalogUtil.ICEBERG_CATALOG_TYPE; + import org.apache.amoro.AmoroTable; import org.apache.amoro.FormatCatalog; +import org.apache.amoro.hive.CachedHiveClientPool; +import org.apache.amoro.hive.HMSClientPool; +import org.apache.amoro.hive.HiveTableTypeUtil; +import org.apache.amoro.properties.CatalogMetaProperties; import org.apache.amoro.table.TableMetaStore; import org.apache.amoro.utils.MixedFormatCatalogUtil; import org.apache.iceberg.Table; @@ -28,9 +34,12 @@ import org.apache.iceberg.catalog.SupportsNamespaces; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NoSuchTableException; +import org.apache.thrift.TException; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; public class IcebergCatalog implements FormatCatalog { @@ -39,14 +48,34 @@ public class IcebergCatalog implements FormatCatalog { private final Catalog icebergCatalog; private final TableMetaStore metaStore; private final Map properties; + private final HMSClientPool hiveClientPool; + /** + * Creates an Iceberg format catalog. + * + *

For a Hive-backed catalog, {@code properties} must contain {@code type=hive}. Production + * callers should normally use {@link IcebergCatalogFactory}, which supplies the metastore type + * explicitly. + */ public IcebergCatalog(Catalog catalog, Map properties, TableMetaStore metaStore) { + this(catalog, properties.get(ICEBERG_CATALOG_TYPE), properties, metaStore); + } + + IcebergCatalog( + Catalog catalog, + String metastoreType, + Map properties, + TableMetaStore metaStore) { this.icebergCatalog = MixedFormatCatalogUtil.buildCacheCatalog(catalog, properties); if (catalog instanceof SupportsNamespaces) { this.asNamespaceCatalog = (SupportsNamespaces) catalog; } this.metaStore = metaStore; this.properties = properties; + this.hiveClientPool = + CatalogMetaProperties.CATALOG_TYPE_HIVE.equalsIgnoreCase(metastoreType) + ? new CachedHiveClientPool(metaStore, properties) + : null; } @Override @@ -89,11 +118,32 @@ public void dropDatabase(String database) { @Override public List listTables(String database) { - return metaStore.doAs( - () -> - icebergCatalog.listTables(Namespace.of(database)).stream() - .map(TableIdentifier::name) - .collect(Collectors.toList())); + List tableNames = + metaStore.doAs( + () -> + icebergCatalog.listTables(Namespace.of(database)).stream() + .map(TableIdentifier::name) + .collect(Collectors.toList())); + if (hiveClientPool == null || tableNames.isEmpty()) { + return tableNames; + } + + Set viewNames = listHiveViewNames(database, tableNames); + return tableNames.stream() + .filter(tableName -> !viewNames.contains(tableName.toLowerCase(Locale.ROOT))) + .collect(Collectors.toList()); + } + + private Set listHiveViewNames(String database, List tableNames) { + try { + return hiveClientPool.run( + client -> HiveTableTypeUtil.listViewNames(client, database, tableNames)); + } catch (TException e) { + throw new RuntimeException("Failed to identify Hive views in database: " + database, e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Interrupted while identifying Hive views", e); + } } @Override diff --git a/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalogFactory.java b/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalogFactory.java index 432fa81b38..683ed00b23 100644 --- a/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalogFactory.java +++ b/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalogFactory.java @@ -42,7 +42,7 @@ public FormatCatalog create( Catalog icebergCatalog = CatalogUtil.buildIcebergCatalog(name, properties, metaStore.getConfiguration()); - return new IcebergCatalog(icebergCatalog, properties, metaStore); + return new IcebergCatalog(icebergCatalog, metastoreType, properties, metaStore); } @Override diff --git a/amoro-format-mixed/amoro-mixed-hive/src/main/java/org/apache/amoro/hive/catalog/MixedHiveCatalog.java b/amoro-format-mixed/amoro-mixed-hive/src/main/java/org/apache/amoro/hive/catalog/MixedHiveCatalog.java index 6e43e0fb81..ae529e83eb 100644 --- a/amoro-format-mixed/amoro-mixed-hive/src/main/java/org/apache/amoro/hive/catalog/MixedHiveCatalog.java +++ b/amoro-format-mixed/amoro-mixed-hive/src/main/java/org/apache/amoro/hive/catalog/MixedHiveCatalog.java @@ -33,6 +33,7 @@ import org.apache.amoro.hive.CachedHiveClientPool; import org.apache.amoro.hive.HMSClient; import org.apache.amoro.hive.HMSClientPool; +import org.apache.amoro.hive.HiveTableTypeUtil; import org.apache.amoro.hive.utils.CompatibleHivePropertyUtil; import org.apache.amoro.hive.utils.HiveSchemaUtil; import org.apache.amoro.hive.utils.HiveTableUtil; @@ -183,14 +184,21 @@ protected TableMeta getMixedTableMeta(TableIdentifier identifier) { throw new NoSuchTableException("load table failed %s.", identifier); } - Map hiveParameters = hiveTable.getParameters(); + if (HiveTableTypeUtil.isView(hiveTable)) { + throw new NoSuchTableException("%s is a Hive view, not a Mixed Hive table.", identifier); + } - String mixedTableRootLocation = hiveParameters.get(MIXED_TABLE_ROOT_LOCATION); + Map hiveParameters = hiveTable.getParameters(); + String mixedTableRootLocation = + hiveParameters == null ? null : hiveParameters.get(MIXED_TABLE_ROOT_LOCATION); if (mixedTableRootLocation == null) { // if hive location ends with /hive, then it's a mixed-hive table. we need to remove /hive to // get root location. // if hive location doesn't end with /hive, then it's a pure-hive table. we can use the // location as root location. + if (hiveTable.getSd() == null || StringUtils.isBlank(hiveTable.getSd().getLocation())) { + throw new NoSuchTableException("table %s does not have a storage location.", identifier); + } String hiveRootLocation = hiveTable.getSd().getLocation(); if (hiveRootLocation.endsWith("/hive")) { mixedTableRootLocation = hiveRootLocation.substring(0, hiveRootLocation.length() - 5); @@ -305,7 +313,8 @@ public List listTables(String database) { hiveTables.stream() .filter( table -> - table.getParameters() != null + !HiveTableTypeUtil.isView(table) + && table.getParameters() != null && CompatibleHivePropertyUtil.propertyAsBoolean( table.getParameters(), HiveTableProperties.MIXED_TABLE_FLAG, @@ -327,8 +336,11 @@ public List listTables(String database) { }); } catch (NoSuchObjectException e) { // pass - } catch (TException | InterruptedException e) { + } catch (TException e) { throw new RuntimeException("Failed to listTables of database :" + database, e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Interrupted while listing tables of database: " + database, e); } return result; } diff --git a/amoro-format-mixed/amoro-mixed-hive/src/main/java/org/apache/amoro/hive/utils/HiveTableUtil.java b/amoro-format-mixed/amoro-mixed-hive/src/main/java/org/apache/amoro/hive/utils/HiveTableUtil.java index 64eb8e1729..2c427b3287 100644 --- a/amoro-format-mixed/amoro-mixed-hive/src/main/java/org/apache/amoro/hive/utils/HiveTableUtil.java +++ b/amoro-format-mixed/amoro-mixed-hive/src/main/java/org/apache/amoro/hive/utils/HiveTableUtil.java @@ -18,7 +18,9 @@ package org.apache.amoro.hive.utils; +import org.apache.amoro.NoSuchTableException; import org.apache.amoro.hive.HMSClientPool; +import org.apache.amoro.hive.HiveTableTypeUtil; import org.apache.amoro.properties.HiveTableProperties; import org.apache.amoro.shade.guava32.com.google.common.collect.Maps; import org.apache.amoro.table.TableIdentifier; @@ -37,8 +39,12 @@ import org.slf4j.LoggerFactory; import java.io.IOException; +import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; public class HiveTableUtil { @@ -61,6 +67,23 @@ public static org.apache.hadoop.hive.metastore.api.Table loadHmsTable( } } + /** + * Loads a physical Hive table and rejects missing tables and Hive views. + * + * @param hiveClient Hive Metastore client pool + * @param tableIdentifier table to load + * @return the physical Hive table + * @throws NoSuchTableException if the table is missing or is a Hive view + */ + public static Table loadPhysicalHmsTable( + HMSClientPool hiveClient, TableIdentifier tableIdentifier) { + Table table = loadHmsTable(hiveClient, tableIdentifier); + if (table == null || HiveTableTypeUtil.isView(table)) { + throw new NoSuchTableException("Hive table does not exist: " + tableIdentifier); + } + return table; + } + public static void persistTable( HMSClientPool hiveClient, org.apache.hadoop.hive.metastore.api.Table tbl) { try { @@ -161,7 +184,8 @@ public static boolean checkExist(HMSClientPool hiveClient, TableIdentifier table } /** - * Gets all the tables in a database. + * Gets all physical tables in a database. Hive views are excluded because they cannot be managed + * as Hive or Mixed Hive tables. * * @param hiveClient Hive client from MixedHiveCatalog * @param database Hive database @@ -169,7 +193,18 @@ public static boolean checkExist(HMSClientPool hiveClient, TableIdentifier table */ public static List getAllHiveTables(HMSClientPool hiveClient, String database) { try { - return hiveClient.run(client -> client.getAllTables(database)); + return hiveClient.run( + client -> { + List tableNames = client.getAllTables(database); + if (tableNames == null || tableNames.isEmpty()) { + return Collections.emptyList(); + } + + Set viewNames = HiveTableTypeUtil.listViewNames(client, database, tableNames); + return tableNames.stream() + .filter(name -> !viewNames.contains(name.toLowerCase(Locale.ROOT))) + .collect(Collectors.toList()); + }); } catch (TException e) { throw new RuntimeException("Failed to get tables of database " + database, e); } catch (InterruptedException e) { diff --git a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/catalog/TestMixedHiveCatalog.java b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/catalog/TestMixedHiveCatalog.java index 973e8b7c2a..e73d1c16bd 100644 --- a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/catalog/TestMixedHiveCatalog.java +++ b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/catalog/TestMixedHiveCatalog.java @@ -23,18 +23,22 @@ import org.apache.amoro.BasicTableTestHelper; import org.apache.amoro.TableFormat; +import org.apache.amoro.TableTestHelper; import org.apache.amoro.catalog.TestMixedCatalog; import org.apache.amoro.hive.TestHMS; import org.apache.amoro.table.MixedTable; import org.apache.amoro.table.TableIdentifier; import org.apache.iceberg.PartitionSpec; import org.apache.iceberg.Table; +import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.thrift.TException; import org.junit.Assert; import org.junit.ClassRule; +import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import java.util.Collections; import java.util.Map; @RunWith(JUnit4.class) @@ -59,6 +63,24 @@ protected PartitionSpec getCreateTableSpec() { return IDENTIFY_SPEC; } + @Test + public void testHiveViewIsNotLoadableAsMixedHiveTable() throws Exception { + String database = TableTestHelper.TEST_DB_NAME; + String viewName = "test_hive_view"; + getMixedFormatCatalog().createDatabase(database); + TEST_HMS.createView(database, viewName, Collections.singletonMap(MIXED_TABLE_FLAG, "true")); + TableIdentifier viewIdentifier = + TableIdentifier.of(getCatalogMeta().getCatalogName(), database, viewName); + + try { + Assert.assertFalse(getMixedFormatCatalog().listTables(database).contains(viewIdentifier)); + Assert.assertThrows( + NoSuchTableException.class, () -> getMixedFormatCatalog().loadTable(viewIdentifier)); + } finally { + TEST_HMS.getHiveClient().dropTable(database, viewName, false, true); + } + } + private void validateMixedHiveTableProperties(TableIdentifier tableIdentifier) throws TException { String dbName = tableIdentifier.getDatabase(); String tbl = tableIdentifier.getTableName(); diff --git a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/IcebergHiveCatalogTestHelper.java b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/IcebergHiveCatalogTestHelper.java index 044a107322..6c9dd63135 100644 --- a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/IcebergHiveCatalogTestHelper.java +++ b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/IcebergHiveCatalogTestHelper.java @@ -23,6 +23,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.hive.HiveCatalog; import java.util.HashMap; import java.util.Map; @@ -43,6 +44,8 @@ protected String getMetastoreType() { } public static IcebergHiveCatalogTestHelper defaultHelper() { - return new IcebergHiveCatalogTestHelper("test_iceberg_catalog", new HashMap<>()); + Map properties = new HashMap<>(); + properties.put(HiveCatalog.LIST_ALL_TABLES, Boolean.TRUE.toString()); + return new IcebergHiveCatalogTestHelper("test_iceberg_catalog", properties); } } diff --git a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/TestIcebergHiveAmoroCatalog.java b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/TestIcebergHiveAmoroCatalog.java index ba42904319..790432fb0b 100644 --- a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/TestIcebergHiveAmoroCatalog.java +++ b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/TestIcebergHiveAmoroCatalog.java @@ -18,14 +18,23 @@ package org.apache.amoro.hive.formats; +import org.apache.amoro.FormatCatalog; +import org.apache.amoro.NoSuchTableException; import org.apache.amoro.formats.AmoroCatalogTestHelper; import org.apache.amoro.formats.TestIcebergAmoroCatalog; import org.apache.amoro.hive.TestHMS; +import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.junit.Assert; import org.junit.ClassRule; +import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import java.io.IOException; +import java.util.HashMap; +import java.util.List; @RunWith(Parameterized.class) public class TestIcebergHiveAmoroCatalog extends TestIcebergAmoroCatalog { @@ -46,4 +55,30 @@ public void setupCatalog() throws IOException { catalogTestHelper.initHiveConf(TEST_HMS.getHiveConf()); super.setupCatalog(); } + + @Test + public void testListTablesExcludesHiveViews() throws Exception { + String database = "view_filter_db"; + String tableName = "iceberg_table"; + String viewName = "hive_view"; + createDatabase(database); + createTable(database, tableName, new HashMap<>()); + TEST_HMS.createView(database, viewName); + + try { + List unfilteredTables = + ((Catalog) originalCatalog).listTables(Namespace.of(database)); + Assert.assertTrue( + unfilteredTables.stream().map(TableIdentifier::name).anyMatch(viewName::equals)); + + List tableNames = ((FormatCatalog) amoroCatalog).listTables(database); + Assert.assertTrue(tableNames.contains(tableName)); + Assert.assertFalse(tableNames.contains(viewName)); + Assert.assertFalse(amoroCatalog.tableExists(database, viewName)); + Assert.assertThrows( + NoSuchTableException.class, () -> amoroCatalog.loadTable(database, viewName)); + } finally { + TEST_HMS.getHiveClient().dropTable(database, viewName, false, true); + } + } } diff --git a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java new file mode 100644 index 0000000000..e0726ff984 --- /dev/null +++ b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java @@ -0,0 +1,147 @@ +/* + * 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.hive.utils; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.apache.amoro.NoSuchTableException; +import org.apache.amoro.client.ClientPool; +import org.apache.amoro.hive.HMSClient; +import org.apache.amoro.hive.HMSClientPool; +import org.apache.amoro.table.TableIdentifier; +import org.apache.hadoop.hive.metastore.TableType; +import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.TableMeta; +import org.apache.thrift.TException; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +public class TestHiveTableUtil { + + @Test + public void testGetAllHiveTablesExcludesViews() throws Exception { + HMSClient client = mock(HMSClient.class); + when(client.getAllTables("database")).thenReturn(Arrays.asList("physical_table", "hive_view")); + when(client.getTableMeta(eq("database"), eq("*"), anyList())) + .thenReturn( + Collections.singletonList( + new TableMeta("database", "hive_view", TableType.VIRTUAL_VIEW.name()))); + + List tableNames = + HiveTableUtil.getAllHiveTables(new TestingHMSClientPool(client), "database"); + + assertEquals(Collections.singletonList("physical_table"), tableNames); + } + + @Test + public void testGetAllHiveTablesFallsBackWhenTableMetaIsUnsupported() throws Exception { + HMSClient client = mock(HMSClient.class); + when(client.getAllTables("database")).thenReturn(Arrays.asList("physical_table", "hive_view")); + when(client.getTableMeta(eq("database"), eq("*"), anyList())) + .thenThrow(new UnsupportedOperationException("not supported")); + when(client.getTableObjectsByName(eq("database"), anyList())) + .thenReturn( + Arrays.asList( + table("physical_table", TableType.EXTERNAL_TABLE), + table("hive_view", TableType.VIRTUAL_VIEW))); + + List tableNames = + HiveTableUtil.getAllHiveTables(new TestingHMSClientPool(client), "database"); + + assertEquals(Collections.singletonList("physical_table"), tableNames); + } + + @Test + public void testGetAllHiveTablesFallsBackWhenTableMetaIsNull() throws Exception { + HMSClient client = mock(HMSClient.class); + when(client.getAllTables("database")).thenReturn(Arrays.asList("physical_table", "hive_view")); + when(client.getTableMeta(eq("database"), eq("*"), anyList())).thenReturn(null); + when(client.getTableObjectsByName(eq("database"), anyList())) + .thenReturn( + Arrays.asList( + table("physical_table", TableType.EXTERNAL_TABLE), + table("hive_view", TableType.MATERIALIZED_VIEW))); + + List tableNames = + HiveTableUtil.getAllHiveTables(new TestingHMSClientPool(client), "database"); + + assertEquals(Collections.singletonList("physical_table"), tableNames); + } + + @Test + public void testLoadPhysicalHmsTableRejectsView() throws Exception { + HMSClient client = mock(HMSClient.class); + when(client.getTable("database", "hive_view")) + .thenReturn(table("hive_view", TableType.VIRTUAL_VIEW)); + TableIdentifier identifier = TableIdentifier.of("catalog", "database", "hive_view"); + + assertThrows( + NoSuchTableException.class, + () -> HiveTableUtil.loadPhysicalHmsTable(new TestingHMSClientPool(client), identifier)); + } + + @Test + public void testLoadPhysicalHmsTable() throws Exception { + HMSClient client = mock(HMSClient.class); + Table physicalTable = table("physical_table", TableType.EXTERNAL_TABLE); + when(client.getTable("database", "physical_table")).thenReturn(physicalTable); + TableIdentifier identifier = TableIdentifier.of("catalog", "database", "physical_table"); + + assertEquals( + physicalTable, + HiveTableUtil.loadPhysicalHmsTable(new TestingHMSClientPool(client), identifier)); + } + + private static Table table(String name, TableType tableType) { + Table table = new Table(); + table.setDbName("database"); + table.setTableName(name); + table.setTableType(tableType.name()); + return table; + } + + private static class TestingHMSClientPool implements HMSClientPool { + + private final HMSClient client; + + private TestingHMSClientPool(HMSClient client) { + this.client = client; + } + + @Override + public R run(ClientPool.Action action) + throws TException, InterruptedException { + return action.run(client); + } + + @Override + public R run(ClientPool.Action action, boolean retry) + throws TException, InterruptedException { + return action.run(client); + } + } +} From adce19bebe1da43839245a91a7260219925a73f9 Mon Sep 17 00:00:00 2001 From: caosurong Date: Tue, 1 Sep 2026 20:01:20 +0800 Subject: [PATCH 2/3] Fix Hive view type compatibility across versions --- .../java/org/apache/amoro/hive/HiveTableTypeUtil.java | 10 +++++----- .../org/apache/amoro/hive/TestHiveTableTypeUtil.java | 2 +- .../org/apache/amoro/hive/utils/TestHiveTableUtil.java | 8 ++++++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java b/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java index be9bfa4ba0..0bf96f499d 100644 --- a/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java +++ b/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java @@ -18,7 +18,6 @@ package org.apache.amoro.hive; -import org.apache.hadoop.hive.metastore.TableType; import org.apache.hadoop.hive.metastore.api.Table; import org.apache.hadoop.hive.metastore.api.TableMeta; import org.apache.thrift.TException; @@ -34,9 +33,10 @@ /** Utilities for classifying Hive Metastore table objects. */ public final class HiveTableTypeUtil { + private static final String VIRTUAL_VIEW_TYPE = "VIRTUAL_VIEW"; + private static final String MATERIALIZED_VIEW_TYPE = "MATERIALIZED_VIEW"; private static final List VIEW_TYPES = - Collections.unmodifiableList( - Arrays.asList(TableType.VIRTUAL_VIEW.name(), TableType.MATERIALIZED_VIEW.name())); + Collections.unmodifiableList(Arrays.asList(VIRTUAL_VIEW_TYPE, MATERIALIZED_VIEW_TYPE)); private HiveTableTypeUtil() {} @@ -106,7 +106,7 @@ private static Set listViewNamesFromTables( } private static boolean isViewType(String tableType) { - return TableType.VIRTUAL_VIEW.name().equalsIgnoreCase(tableType) - || TableType.MATERIALIZED_VIEW.name().equalsIgnoreCase(tableType); + return VIRTUAL_VIEW_TYPE.equalsIgnoreCase(tableType) + || MATERIALIZED_VIEW_TYPE.equalsIgnoreCase(tableType); } } diff --git a/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java b/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java index e7badc7b69..3b1e9167f3 100644 --- a/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java +++ b/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java @@ -31,7 +31,7 @@ public class TestHiveTableTypeUtil { @Test public void testViewTypes() { assertTrue(HiveTableTypeUtil.isView(tableWithType(TableType.VIRTUAL_VIEW.name()))); - assertTrue(HiveTableTypeUtil.isView(tableWithType(TableType.MATERIALIZED_VIEW.name()))); + assertTrue(HiveTableTypeUtil.isView(tableWithType("MATERIALIZED_VIEW"))); assertFalse(HiveTableTypeUtil.isView(tableWithType(TableType.EXTERNAL_TABLE.name()))); assertFalse(HiveTableTypeUtil.isView(tableWithType(null))); assertTrue( diff --git a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java index e0726ff984..e93a7aeef4 100644 --- a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java +++ b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java @@ -84,7 +84,7 @@ public void testGetAllHiveTablesFallsBackWhenTableMetaIsNull() throws Exception .thenReturn( Arrays.asList( table("physical_table", TableType.EXTERNAL_TABLE), - table("hive_view", TableType.MATERIALIZED_VIEW))); + table("hive_view", "MATERIALIZED_VIEW"))); List tableNames = HiveTableUtil.getAllHiveTables(new TestingHMSClientPool(client), "database"); @@ -117,10 +117,14 @@ public void testLoadPhysicalHmsTable() throws Exception { } private static Table table(String name, TableType tableType) { + return table(name, tableType.name()); + } + + private static Table table(String name, String tableType) { Table table = new Table(); table.setDbName("database"); table.setTableName(name); - table.setTableType(tableType.name()); + table.setTableType(tableType); return table; } From 0aec5001edb3558c17c8f8ac5f4dce152c645a15 Mon Sep 17 00:00:00 2001 From: csurong Date: Wed, 9 Sep 2026 10:57:37 +0800 Subject: [PATCH 3/3] Simplify Hive view lookup and avoid redundant metastore calls --- .../java/org/apache/amoro/hive/HMSClient.java | 7 +- .../apache/amoro/hive/HiveTableTypeUtil.java | 29 +------- .../amoro/hive/TestHiveTableTypeUtil.java | 34 +++++++++ .../amoro/formats/iceberg/IcebergCatalog.java | 5 ++ .../formats/iceberg/TestIcebergCatalog.java | 72 +++++++++++++++++++ .../formats/IcebergHiveCatalogTestHelper.java | 5 +- .../formats/TestIcebergHiveAmoroCatalog.java | 21 +++++- .../amoro/hive/utils/TestHiveTableUtil.java | 48 ++++++------- 8 files changed, 158 insertions(+), 63 deletions(-) create mode 100644 amoro-format-iceberg/src/test/java/org/apache/amoro/formats/iceberg/TestIcebergCatalog.java diff --git a/amoro-common/src/main/java/org/apache/amoro/hive/HMSClient.java b/amoro-common/src/main/java/org/apache/amoro/hive/HMSClient.java index f4c7751772..abb2aa92d5 100644 --- a/amoro-common/src/main/java/org/apache/amoro/hive/HMSClient.java +++ b/amoro-common/src/main/java/org/apache/amoro/hive/HMSClient.java @@ -97,10 +97,7 @@ void alterPartitions( * @param tablePattern Hive table name or pattern * @param tableTypes Hive table types to include * @return matching lightweight table metadata - * @throws UnsupportedOperationException if the client implementation does not support this API */ - default List getTableMeta( - String databasePattern, String tablePattern, List tableTypes) throws TException { - throw new UnsupportedOperationException("Getting Hive table metadata is not supported"); - } + List getTableMeta(String databasePattern, String tablePattern, List tableTypes) + throws TException; } diff --git a/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java b/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java index 0bf96f499d..38e4f8274a 100644 --- a/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java +++ b/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java @@ -58,9 +58,6 @@ public static boolean isView(TableMeta table) { /** * Returns the normalized names of Hive views among the candidate tables. * - *

The lightweight table metadata API is preferred. Clients that do not implement that API fall - * back to loading the candidate table objects. - * * @param client Hive Metastore client * @param database database containing the candidate tables * @param candidateTableNames table names to inspect @@ -72,15 +69,10 @@ public static Set listViewNames( return Collections.emptySet(); } - List views; - try { - views = client.getTableMeta(database, "*", viewTypes()); - } catch (UnsupportedOperationException e) { - return listViewNamesFromTables(client, database, candidateTableNames); - } - + List views = client.getTableMeta(database, "*", viewTypes()); if (views == null) { - return listViewNamesFromTables(client, database, candidateTableNames); + throw new IllegalStateException( + "Hive Metastore returned null while loading table metadata from database: " + database); } return views.stream() .filter(HiveTableTypeUtil::isView) @@ -90,21 +82,6 @@ public static Set listViewNames( .collect(Collectors.toCollection(HashSet::new)); } - private static Set listViewNamesFromTables( - HMSClient client, String database, List candidateTableNames) throws TException { - List

tables = client.getTableObjectsByName(database, candidateTableNames); - if (tables == null) { - throw new IllegalStateException( - "Hive Metastore returned null while loading table objects from database: " + database); - } - return tables.stream() - .filter(HiveTableTypeUtil::isView) - .map(Table::getTableName) - .filter(name -> name != null) - .map(name -> name.toLowerCase(Locale.ROOT)) - .collect(Collectors.toCollection(HashSet::new)); - } - private static boolean isViewType(String tableType) { return VIRTUAL_VIEW_TYPE.equalsIgnoreCase(tableType) || MATERIALIZED_VIEW_TYPE.equalsIgnoreCase(tableType); diff --git a/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java b/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java index 3b1e9167f3..50e55b77ac 100644 --- a/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java +++ b/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java @@ -18,14 +18,22 @@ package org.apache.amoro.hive; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; import org.apache.hadoop.hive.metastore.TableType; import org.apache.hadoop.hive.metastore.api.Table; import org.apache.hadoop.hive.metastore.api.TableMeta; import org.junit.jupiter.api.Test; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; + public class TestHiveTableTypeUtil { @Test @@ -38,6 +46,32 @@ public void testViewTypes() { HiveTableTypeUtil.isView(new TableMeta("database", "view", TableType.VIRTUAL_VIEW.name()))); } + @Test + public void testListViewNames() throws Exception { + HMSClient client = mock(HMSClient.class); + when(client.getTableMeta("database", "*", HiveTableTypeUtil.viewTypes())) + .thenReturn( + Arrays.asList( + new TableMeta("database", "Hive_View", "VIRTUAL_VIEW"), + new TableMeta("database", "Materialized_View", "MATERIALIZED_VIEW"), + new TableMeta("database", "physical_table", "EXTERNAL_TABLE"))); + + assertEquals( + new HashSet<>(Arrays.asList("hive_view", "materialized_view")), + HiveTableTypeUtil.listViewNames( + client, "database", Arrays.asList("Hive_View", "Materialized_View", "physical_table"))); + } + + @Test + public void testListViewNamesSkipsEmptyCandidates() throws Exception { + HMSClient client = mock(HMSClient.class); + + assertTrue( + HiveTableTypeUtil.listViewNames(client, "database", Collections.emptyList()).isEmpty()); + assertTrue(HiveTableTypeUtil.listViewNames(client, "database", null).isEmpty()); + verifyNoInteractions(client); + } + private static Table tableWithType(String tableType) { Table table = new Table(); table.setTableType(tableType); diff --git a/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalog.java b/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalog.java index 9125eec7fb..956f921c68 100644 --- a/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalog.java +++ b/amoro-format-iceberg/src/main/java/org/apache/amoro/formats/iceberg/IcebergCatalog.java @@ -28,6 +28,7 @@ import org.apache.amoro.properties.CatalogMetaProperties; import org.apache.amoro.table.TableMetaStore; import org.apache.amoro.utils.MixedFormatCatalogUtil; +import org.apache.amoro.utils.PropertyUtil; import org.apache.iceberg.Table; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; @@ -44,6 +45,9 @@ public class IcebergCatalog implements FormatCatalog { + // Matches HiveCatalog without a compile-time dependency on iceberg-hive-metastore. + private static final String LIST_ALL_TABLES = "list-all-tables"; + private SupportsNamespaces asNamespaceCatalog; private final Catalog icebergCatalog; private final TableMetaStore metaStore; @@ -74,6 +78,7 @@ public IcebergCatalog(Catalog catalog, Map properties, TableMeta this.properties = properties; this.hiveClientPool = CatalogMetaProperties.CATALOG_TYPE_HIVE.equalsIgnoreCase(metastoreType) + && PropertyUtil.propertyAsBoolean(properties, LIST_ALL_TABLES, false) ? new CachedHiveClientPool(metaStore, properties) : null; } diff --git a/amoro-format-iceberg/src/test/java/org/apache/amoro/formats/iceberg/TestIcebergCatalog.java b/amoro-format-iceberg/src/test/java/org/apache/amoro/formats/iceberg/TestIcebergCatalog.java new file mode 100644 index 0000000000..2926c24025 --- /dev/null +++ b/amoro-format-iceberg/src/test/java/org/apache/amoro/formats/iceberg/TestIcebergCatalog.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.formats.iceberg; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import org.apache.amoro.properties.CatalogMetaProperties; +import org.apache.amoro.table.TableMetaStore; +import org.apache.iceberg.CatalogProperties; +import org.apache.iceberg.catalog.Catalog; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.Callable; + +public class TestIcebergCatalog { + + @Test + public void testListTablesSkipsViewLookupByDefault() { + assertListTablesSkipsViewLookup(new HashMap<>()); + } + + @Test + public void testListTablesSkipsViewLookupWhenListAllTablesIsFalse() { + Map properties = new HashMap<>(); + properties.put("list-all-tables", "false"); + assertListTablesSkipsViewLookup(properties); + } + + private void assertListTablesSkipsViewLookup(Map properties) { + properties.put(CatalogProperties.CACHE_ENABLED, "false"); + Catalog catalog = mock(Catalog.class); + TableMetaStore metaStore = mock(TableMetaStore.class); + when(metaStore.doAs(any())) + .thenAnswer(invocation -> invocation.>getArgument(0).call()); + when(catalog.listTables(Namespace.of("database"))) + .thenReturn(Collections.singletonList(TableIdentifier.of("database", "iceberg_table"))); + IcebergCatalog amoroCatalog = + new IcebergCatalog(catalog, CatalogMetaProperties.CATALOG_TYPE_HIVE, properties, metaStore); + + assertEquals(Collections.singletonList("iceberg_table"), amoroCatalog.listTables("database")); + + verify(catalog).listTables(Namespace.of("database")); + verify(metaStore).doAs(any()); + verifyNoMoreInteractions(catalog, metaStore); + } +} diff --git a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/IcebergHiveCatalogTestHelper.java b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/IcebergHiveCatalogTestHelper.java index 6c9dd63135..044a107322 100644 --- a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/IcebergHiveCatalogTestHelper.java +++ b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/IcebergHiveCatalogTestHelper.java @@ -23,7 +23,6 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.conf.HiveConf; import org.apache.iceberg.CatalogProperties; -import org.apache.iceberg.hive.HiveCatalog; import java.util.HashMap; import java.util.Map; @@ -44,8 +43,6 @@ protected String getMetastoreType() { } public static IcebergHiveCatalogTestHelper defaultHelper() { - Map properties = new HashMap<>(); - properties.put(HiveCatalog.LIST_ALL_TABLES, Boolean.TRUE.toString()); - return new IcebergHiveCatalogTestHelper("test_iceberg_catalog", properties); + return new IcebergHiveCatalogTestHelper("test_iceberg_catalog", new HashMap<>()); } } diff --git a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/TestIcebergHiveAmoroCatalog.java b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/TestIcebergHiveAmoroCatalog.java index 790432fb0b..58adea435d 100644 --- a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/TestIcebergHiveAmoroCatalog.java +++ b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/TestIcebergHiveAmoroCatalog.java @@ -26,6 +26,7 @@ import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.hive.HiveCatalog; import org.junit.Assert; import org.junit.ClassRule; import org.junit.Test; @@ -33,6 +34,7 @@ import org.junit.runners.Parameterized; import java.io.IOException; +import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -47,7 +49,15 @@ public TestIcebergHiveAmoroCatalog(AmoroCatalogTestHelper amoroCatalogTestHel @Parameterized.Parameters(name = "{0}") public static Object[] parameters() { - return new Object[] {IcebergHiveCatalogTestHelper.defaultHelper()}; + return new Object[] { + IcebergHiveCatalogTestHelper.defaultHelper(), + new IcebergHiveCatalogTestHelper( + "test_iceberg_catalog_list_all_false", + new HashMap<>(Collections.singletonMap(HiveCatalog.LIST_ALL_TABLES, "false"))), + new IcebergHiveCatalogTestHelper( + "test_iceberg_catalog_list_all_true", + new HashMap<>(Collections.singletonMap(HiveCatalog.LIST_ALL_TABLES, "true"))) + }; } @Override @@ -68,7 +78,14 @@ public void testListTablesExcludesHiveViews() throws Exception { try { List unfilteredTables = ((Catalog) originalCatalog).listTables(Namespace.of(database)); - Assert.assertTrue( + boolean listAllTables = + Boolean.parseBoolean( + catalogTestHelper + .getCatalogMeta() + .getCatalogProperties() + .getOrDefault(HiveCatalog.LIST_ALL_TABLES, HiveCatalog.LIST_ALL_TABLES_DEFAULT)); + Assert.assertEquals( + listAllTables, unfilteredTables.stream().map(TableIdentifier::name).anyMatch(viewName::equals)); List tableNames = ((FormatCatalog) amoroCatalog).listTables(database); diff --git a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java index e93a7aeef4..02f53a08ba 100644 --- a/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java +++ b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/utils/TestHiveTableUtil.java @@ -19,10 +19,13 @@ package org.apache.amoro.hive.utils; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.apache.amoro.NoSuchTableException; @@ -58,38 +61,35 @@ public void testGetAllHiveTablesExcludesViews() throws Exception { } @Test - public void testGetAllHiveTablesFallsBackWhenTableMetaIsUnsupported() throws Exception { + public void testGetAllHiveTablesPropagatesTableMetaFailure() throws Exception { HMSClient client = mock(HMSClient.class); when(client.getAllTables("database")).thenReturn(Arrays.asList("physical_table", "hive_view")); - when(client.getTableMeta(eq("database"), eq("*"), anyList())) - .thenThrow(new UnsupportedOperationException("not supported")); - when(client.getTableObjectsByName(eq("database"), anyList())) - .thenReturn( - Arrays.asList( - table("physical_table", TableType.EXTERNAL_TABLE), - table("hive_view", TableType.VIRTUAL_VIEW))); + TException failure = new TException("Failed to get table metadata"); + when(client.getTableMeta(eq("database"), eq("*"), anyList())).thenThrow(failure); - List tableNames = - HiveTableUtil.getAllHiveTables(new TestingHMSClientPool(client), "database"); + RuntimeException exception = + assertThrows( + RuntimeException.class, + () -> HiveTableUtil.getAllHiveTables(new TestingHMSClientPool(client), "database")); - assertEquals(Collections.singletonList("physical_table"), tableNames); + assertSame(failure, exception.getCause()); + verify(client, never()).getTableObjectsByName(eq("database"), anyList()); } @Test - public void testGetAllHiveTablesFallsBackWhenTableMetaIsNull() throws Exception { + public void testGetAllHiveTablesRejectsNullTableMeta() throws Exception { HMSClient client = mock(HMSClient.class); when(client.getAllTables("database")).thenReturn(Arrays.asList("physical_table", "hive_view")); when(client.getTableMeta(eq("database"), eq("*"), anyList())).thenReturn(null); - when(client.getTableObjectsByName(eq("database"), anyList())) - .thenReturn( - Arrays.asList( - table("physical_table", TableType.EXTERNAL_TABLE), - table("hive_view", "MATERIALIZED_VIEW"))); + IllegalStateException exception = + assertThrows( + IllegalStateException.class, + () -> HiveTableUtil.getAllHiveTables(new TestingHMSClientPool(client), "database")); - List tableNames = - HiveTableUtil.getAllHiveTables(new TestingHMSClientPool(client), "database"); - - assertEquals(Collections.singletonList("physical_table"), tableNames); + assertEquals( + "Hive Metastore returned null while loading table metadata from database: database", + exception.getMessage()); + verify(client, never()).getTableObjectsByName(eq("database"), anyList()); } @Test @@ -117,14 +117,10 @@ public void testLoadPhysicalHmsTable() throws Exception { } private static Table table(String name, TableType tableType) { - return table(name, tableType.name()); - } - - private static Table table(String name, String tableType) { Table table = new Table(); table.setDbName("database"); table.setTableName(name); - table.setTableType(tableType); + table.setTableType(tableType.name()); return table; }