This is an automated email from the ASF dual-hosted git repository.
czy006 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/amoro.git
The following commit(s) were added to refs/heads/master by this push:
new 1909b950b [AMORO-4350][catalog] Fix Hive view handling in table
management (#4351)
1909b950b is described below
commit 1909b950b0733fdfec5d5bc7532f3a4b6a86e5e7
Author: csurong <[email protected]>
AuthorDate: Tue Sep 15 21:20:52 2026 +0800
[AMORO-4350][catalog] Fix Hive view handling in table management (#4351)
* Fix Hive view filtering in table management
* Fix Hive view type compatibility across versions
* Simplify Hive view lookup and avoid redundant metastore calls
---------
Co-authored-by: caosurong <surong.cao>
Co-authored-by: ConradJam <[email protected]>
---
.../dashboard/controller/TableController.java | 10 +-
.../main/java/org/apache/amoro/hive/HMSClient.java | 12 ++
.../java/org/apache/amoro/hive/HMSClientImpl.java | 7 +
.../org/apache/amoro/hive/HiveTableTypeUtil.java | 89 +++++++++++++
.../test/java/org/apache/amoro/hive/TestHMS.java | 36 +++++
.../apache/amoro/hive/TestHiveTableTypeUtil.java | 80 +++++++++++
.../amoro/formats/iceberg/IcebergCatalog.java | 65 ++++++++-
.../formats/iceberg/IcebergCatalogFactory.java | 2 +-
.../amoro/formats/iceberg/TestIcebergCatalog.java | 72 ++++++++++
.../amoro/hive/catalog/MixedHiveCatalog.java | 20 ++-
.../org/apache/amoro/hive/utils/HiveTableUtil.java | 39 +++++-
.../amoro/hive/catalog/TestMixedHiveCatalog.java | 22 +++
.../hive/formats/TestIcebergHiveAmoroCatalog.java | 54 +++++++-
.../apache/amoro/hive/utils/TestHiveTableUtil.java | 147 +++++++++++++++++++++
14 files changed, 641 insertions(+), 14 deletions(-)
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 772e8d0dd..087acdbc5 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 class TableController {
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<AMSColumnInfo> schema =
transformHiveSchemaToAMSColumnInfo(hiveTable.getSd().getCols());
List<AMSColumnInfo> partitionColumnInfos =
transformHiveSchemaToAMSColumnInfo(hiveTable.getPartitionKeys());
@@ -727,6 +732,9 @@ public class TableController {
}
private List<AMSColumnInfo>
transformHiveSchemaToAMSColumnInfo(List<FieldSchema> 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 140324abc..abb2aa92d 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.EnvironmentContext;
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 @@ public interface HMSClient {
InvocationTargetException, ClassNotFoundException;
List<Table> getTableObjectsByName(String dbName, List<String> 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<TableMeta> getTableMeta(String databasePattern, String tablePattern,
List<String> 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 85ddb6833..a1d49d014 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.EnvironmentContext;
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 class HMSClientImpl implements HMSClient {
throws TException {
return getClient().getTableObjectsByName(dbName, tableNames);
}
+
+ @Override
+ public List<TableMeta> getTableMeta(
+ String databasePattern, String tablePattern, List<String> 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 000000000..38e4f8274
--- /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<String> 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<String> 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<String> listViewNames(
+ HMSClient client, String database, List<String> candidateTableNames)
throws TException {
+ if (candidateTableNames == null || candidateTableNames.isEmpty()) {
+ return Collections.emptySet();
+ }
+
+ List<TableMeta> 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 b7f2efaf8..b61e5f9a5 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 @@ package org.apache.amoro.hive;
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 org.slf4j.LoggerFactory;
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 class TestHMS extends ExternalResource {
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<String, String>
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 000000000..50e55b77a
--- /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 fbe1ff373..956f921c6 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<String, String> properties;
+ private final HMSClientPool hiveClientPool;
+ /**
+ * Creates an Iceberg format catalog.
+ *
+ * <p>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<String, String> properties,
TableMetaStore metaStore) {
+ this(catalog, properties.get(ICEBERG_CATALOG_TYPE), properties, metaStore);
+ }
+
+ IcebergCatalog(
+ Catalog catalog,
+ String metastoreType,
+ Map<String, String> 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 class IcebergCatalog implements FormatCatalog {
@Override
public List<String> listTables(String database) {
- return metaStore.doAs(
- () ->
- icebergCatalog.listTables(Namespace.of(database)).stream()
- .map(TableIdentifier::name)
- .collect(Collectors.toList()));
+ List<String> tableNames =
+ metaStore.doAs(
+ () ->
+ icebergCatalog.listTables(Namespace.of(database)).stream()
+ .map(TableIdentifier::name)
+ .collect(Collectors.toList()));
+ if (hiveClientPool == null || tableNames.isEmpty()) {
+ return tableNames;
+ }
+
+ Set<String> viewNames = listHiveViewNames(database, tableNames);
+ return tableNames.stream()
+ .filter(tableName ->
!viewNames.contains(tableName.toLowerCase(Locale.ROOT)))
+ .collect(Collectors.toList());
+ }
+
+ private Set<String> listHiveViewNames(String database, List<String>
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 432fa81b3..683ed00b2 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 class IcebergCatalogFactory implements
FormatCatalogFactory {
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 000000000..2926c2402
--- /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<String, String> properties = new HashMap<>();
+ properties.put("list-all-tables", "false");
+ assertListTablesSkipsViewLookup(properties);
+ }
+
+ private void assertListTablesSkipsViewLookup(Map<String, String> properties)
{
+ properties.put(CatalogProperties.CACHE_ENABLED, "false");
+ Catalog catalog = mock(Catalog.class);
+ TableMetaStore metaStore = mock(TableMetaStore.class);
+ when(metaStore.doAs(any()))
+ .thenAnswer(invocation ->
invocation.<Callable<?>>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 6e43e0fb8..ae529e83e 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.api.TableMeta;
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 @@ public class MixedHiveCatalog implements
MixedFormatCatalog {
throw new NoSuchTableException("load table failed %s.", identifier);
}
- Map<String, String> 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<String, String> 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 class MixedHiveCatalog implements MixedFormatCatalog
{
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 class MixedHiveCatalog implements
MixedFormatCatalog {
});
} 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 64eb8e172..2c427b328 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.Logger;
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 class HiveTableUtil {
}
}
+ /**
+ * 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 class HiveTableUtil {
}
/**
- * 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 class HiveTableUtil {
*/
public static List<String> getAllHiveTables(HMSClientPool hiveClient, String
database) {
try {
- return hiveClient.run(client -> client.getAllTables(database));
+ return hiveClient.run(
+ client -> {
+ List<String> tableNames = client.getAllTables(database);
+ if (tableNames == null || tableNames.isEmpty()) {
+ return Collections.emptyList();
+ }
+
+ Set<String> 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 973e8b7c2..e73d1c16b 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 static
org.apache.amoro.properties.HiveTableProperties.MIXED_TABLE_ROOT_L
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 @@ public class TestMixedHiveCatalog extends TestMixedCatalog {
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 ba4290431..58adea435 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 class TestIcebergHiveAmoroCatalog extends
TestIcebergAmoroCatalog {
@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 class TestIcebergHiveAmoroCatalog extends
TestIcebergAmoroCatalog {
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<TableIdentifier> 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<String> 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 000000000..02f53a08b
--- /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<String> 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> R run(ClientPool.Action<R, HMSClient, TException> action)
+ throws TException, InterruptedException {
+ return action.run(client);
+ }
+
+ @Override
+ public <R> R run(ClientPool.Action<R, HMSClient, TException> action,
boolean retry)
+ throws TException, InterruptedException {
+ return action.run(client);
+ }
+ }
+}
