gaborkaszab commented on code in PR #11597:
URL: https://github.com/apache/iceberg/pull/11597#discussion_r1857260261


##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -412,6 +412,28 @@ private void validateTableIsIcebergTableOrView(
     }
   }
 
+  @Override
+  public boolean tableExists(TableIdentifier identifier) {
+    if (!isValidIdentifier(identifier)) {

Review Comment:
   +1 to Szehon's point. I think the original tableExists returned true for 
metadata tables too. I doesn't seem to be covered here.



##########
hive-metastore/src/test/java/org/apache/iceberg/hive/HiveTableTest.java:
##########
@@ -386,6 +386,12 @@ public void 
testHiveTableAndIcebergTableWithSameName(TableType tableType)
 
     assertThat(catalog.tableExists(TABLE_IDENTIFIER)).isTrue();
     HIVE_METASTORE_EXTENSION.metastoreClient().dropTable(DB_NAME, 
hiveTableName);
+

Review Comment:
   Unrelated, but this test also verifies that there is an existing table 
remaining from the previous test. All these tests would be much simpler if they 
cleaned up after themselves.



##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveCatalog.java:
##########
@@ -412,6 +412,28 @@ private void validateTableIsIcebergTableOrView(
     }
   }
 
+  @Override
+  public boolean tableExists(TableIdentifier identifier) {
+    if (!isValidIdentifier(identifier)) {
+      return false;
+    }
+
+    String database = identifier.namespace().level(0);
+    try {
+      Table table = clients.run(client -> client.getTable(database, 
identifier.name()));
+      HiveOperationsBase.validateTableIsIceberg(table, fullTableName(name, 
identifier));
+      return true;
+    } catch (NoSuchTableException | NoSuchObjectException e) {
+      return false;

Review Comment:
   I think the behaviour on this area is unchanged with this PR. This function 
returned false when there was a Hive table with the same name and does so with 
this change too.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to