Throw AlreadyExistsException if the table is a view.

apache · Nov 1, 2023 · 3508dcb · 3508dcb
1 parent 6e6fefa
commit 3508dcb
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 16 deletions.
diff --git a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalogUtil.java b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalogUtil.java
@@ -25,6 +25,7 @@
 import org.apache.iceberg.BaseMetastoreTableOperations;
 import org.apache.iceberg.ClientPool;
 import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.AlreadyExistsException;
 import org.apache.iceberg.exceptions.NoSuchIcebergTableException;
 import org.apache.iceberg.exceptions.NoSuchIcebergViewException;
 import org.apache.thrift.TException;
@@ -78,10 +79,13 @@ static void validateTableIsIcebergView(Table table, String fullName) {
   }
 
   static void validateTableIsIceberg(Table table, String fullName) {
+    if (table.getTableType().equalsIgnoreCase(TableType.VIRTUAL_VIEW.name())) {
+      throw new AlreadyExistsException(
+          "View with same name already exists: %s.%s", table.getDbName(), table.getTableName());
+    }
     String tableType = table.getParameters().get(BaseMetastoreTableOperations.TABLE_TYPE_PROP);
     NoSuchIcebergTableException.check(
-        !table.getTableType().equalsIgnoreCase(TableType.VIRTUAL_VIEW.name())
-            && tableType != null
+        tableType != null
             && tableType.equalsIgnoreCase(BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE),
         "Not an iceberg table: %s (type=%s) (tableType=%s)",
         fullName,

diff --git a/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java b/hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
@@ -47,8 +47,6 @@
 import org.apache.iceberg.SortOrderParser;
 import org.apache.iceberg.TableMetadata;
 import org.apache.iceberg.TableProperties;
-import org.apache.iceberg.catalog.Namespace;
-import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.exceptions.AlreadyExistsException;
 import org.apache.iceberg.exceptions.CommitFailedException;
 import org.apache.iceberg.exceptions.CommitStateUnknownException;
@@ -139,18 +137,6 @@ protected HiveTableOperations(
             HiveCatalogUtil.HIVE_TABLE_PROPERTY_MAX_SIZE_DEFAULT);
   }
 
-  @Override
-  public TableMetadata current() {
-    if (HiveCatalogUtil.isTableWithTypeExists(
-        metaClients,
-        TableIdentifier.of(Namespace.of(database), tableName),
-        TableType.VIRTUAL_VIEW)) {
-      throw new AlreadyExistsException(
-          "View with same name already exists: %s.%s", database, tableName);
-    }
-    return super.current();
-  }
-
   @Override
   protected String tableName() {
     return fullName;

diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveViewCatalog.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveViewCatalog.java
@@ -22,6 +22,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.util.Collections;
+import org.apache.iceberg.Transaction;
 import org.apache.iceberg.catalog.Catalog;
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.exceptions.AlreadyExistsException;
@@ -162,4 +163,35 @@ public void renameTableTargetAlreadyExistsAsView() {
     assertThatThrownBy(() -> tableCatalog().renameTable(tableIdentifier, viewIdentifier))
         .hasMessageContaining("new table ns.view already exists");
   }
+
+  @Override
+  @Test
+  public void createTableViaTransactionThatAlreadyExistsAsView() {
+    Assumptions.assumeThat(tableCatalog())
+        .as("Only valid for catalogs that support tables")
+        .isNotNull();
+
+    TableIdentifier viewIdentifier = TableIdentifier.of("ns", "view");
+
+    if (requiresNamespaceCreate()) {
+      catalog().createNamespace(viewIdentifier.namespace());
+    }
+
+    assertThat(catalog().viewExists(viewIdentifier)).as("View should not exist").isFalse();
+
+    Transaction transaction = tableCatalog().buildTable(viewIdentifier, SCHEMA).createTransaction();
+
+    catalog()
+        .buildView(viewIdentifier)
+        .withSchema(SCHEMA)
+        .withDefaultNamespace(viewIdentifier.namespace())
+        .withQuery("spark", "select * from ns.tbl")
+        .create();
+
+    assertThat(catalog().viewExists(viewIdentifier)).as("View should exist").isTrue();
+
+    assertThatThrownBy(transaction::commitTransaction)
+        .isInstanceOf(AlreadyExistsException.class)
+        .hasMessageStartingWith("Table already exists: ns.view");
+  }
 }