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..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 @@ -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,15 @@ 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 + */ + List getTableMeta(String databasePattern, String tablePattern, List tableTypes) + throws TException; } 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..38e4f8274a --- /dev/null +++ b/amoro-common/src/main/java/org/apache/amoro/hive/HiveTableTypeUtil.java @@ -0,0 +1,89 @@ +/* + * 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.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 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(VIRTUAL_VIEW_TYPE, MATERIALIZED_VIEW_TYPE)); + + 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. + * + * @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 = client.getTableMeta(database, "*", viewTypes()); + if (views == null) { + throw new IllegalStateException( + "Hive Metastore returned null while loading table metadata from database: " + database); + } + 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 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/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..50e55b77ac --- /dev/null +++ b/amoro-common/src/test/java/org/apache/amoro/hive/TestHiveTableTypeUtil.java @@ -0,0 +1,80 @@ +/* + * 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.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 + public void testViewTypes() { + assertTrue(HiveTableTypeUtil.isView(tableWithType(TableType.VIRTUAL_VIEW.name()))); + assertTrue(HiveTableTypeUtil.isView(tableWithType("MATERIALIZED_VIEW"))); + assertFalse(HiveTableTypeUtil.isView(tableWithType(TableType.EXTERNAL_TABLE.name()))); + assertFalse(HiveTableTypeUtil.isView(tableWithType(null))); + assertTrue( + 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); + 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..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 @@ -18,35 +18,69 @@ 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.amoro.utils.PropertyUtil; import org.apache.iceberg.Table; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; 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 { + // 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; 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) + && PropertyUtil.propertyAsBoolean(properties, LIST_ALL_TABLES, false) + ? new CachedHiveClientPool(metaStore, properties) + : null; } @Override @@ -89,11 +123,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-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/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/TestIcebergHiveAmoroCatalog.java b/amoro-format-mixed/amoro-mixed-hive/src/test/java/org/apache/amoro/hive/formats/TestIcebergHiveAmoroCatalog.java index ba42904319..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 @@ -18,14 +18,25 @@ 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.apache.iceberg.hive.HiveCatalog; +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.Collections; +import java.util.HashMap; +import java.util.List; @RunWith(Parameterized.class) public class TestIcebergHiveAmoroCatalog extends TestIcebergAmoroCatalog { @@ -38,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 @@ -46,4 +65,37 @@ 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)); + 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); + 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..02f53a08ba --- /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.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; +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 testGetAllHiveTablesPropagatesTableMetaFailure() throws Exception { + HMSClient client = mock(HMSClient.class); + when(client.getAllTables("database")).thenReturn(Arrays.asList("physical_table", "hive_view")); + TException failure = new TException("Failed to get table metadata"); + when(client.getTableMeta(eq("database"), eq("*"), anyList())).thenThrow(failure); + + RuntimeException exception = + assertThrows( + RuntimeException.class, + () -> HiveTableUtil.getAllHiveTables(new TestingHMSClientPool(client), "database")); + + assertSame(failure, exception.getCause()); + verify(client, never()).getTableObjectsByName(eq("database"), anyList()); + } + + @Test + 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); + IllegalStateException exception = + assertThrows( + IllegalStateException.class, + () -> HiveTableUtil.getAllHiveTables(new TestingHMSClientPool(client), "database")); + + assertEquals( + "Hive Metastore returned null while loading table metadata from database: database", + exception.getMessage()); + verify(client, never()).getTableObjectsByName(eq("database"), anyList()); + } + + @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); + } + } +}