(iceberg) branch main updated: Core: Fix loading a table in CachingCatalog with Metadata table name (#11738)

[etudenhoefner]

This is an automated email from the ASF dual-hosted git repository.

etudenhoefner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git


The following commit(s) were added to refs/heads/main by this push:
     new cf8b354c43 Core: Fix loading a table in CachingCatalog with Metadata 
table name (#11738)
cf8b354c43 is described below

commit cf8b354c43d6768b2dd7f72b40421a2a67487520
Author: gaborkaszab <gaborkas...@cloudera.com>
AuthorDate: Tue Jan 14 17:27:48 2025 +0100

    Core: Fix loading a table in CachingCatalog with Metadata table name 
(#11738)
    
    If a regular table had a metadata table name then CachingCatalog throws a
    NoSuchTableException when loading that table.
    
    Co-authored-by: Manu Zhang <owenzhang1...@gmail.com>
---
 .../java/org/apache/iceberg/CachingCatalog.java    | 12 ++++---
 .../apache/iceberg/hadoop/TestCachingCatalog.java  | 39 ++++++++++++++++++++++
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/core/src/main/java/org/apache/iceberg/CachingCatalog.java 
b/core/src/main/java/org/apache/iceberg/CachingCatalog.java
index 1043e3e720..913f1a9482 100644
--- a/core/src/main/java/org/apache/iceberg/CachingCatalog.java
+++ b/core/src/main/java/org/apache/iceberg/CachingCatalog.java
@@ -144,14 +144,16 @@ public class CachingCatalog implements Catalog {
       return cached;
     }
 
-    if (MetadataTableUtils.hasMetadataTableName(canonicalized)) {
+    Table table = tableCache.get(canonicalized, catalog::loadTable);
+
+    if (table instanceof BaseMetadataTable) {
+      // Cache underlying table
       TableIdentifier originTableIdentifier =
           TableIdentifier.of(canonicalized.namespace().levels());
       Table originTable = tableCache.get(originTableIdentifier, 
catalog::loadTable);
 
-      // share TableOperations instance of origin table for all metadata 
tables, so that metadata
-      // table instances are
-      // also refreshed as well when origin table instance is refreshed.
+      // Share TableOperations instance of origin table for all metadata 
tables, so that metadata
+      // table instances are refreshed as well when origin table instance is 
refreshed.
       if (originTable instanceof HasTableOperations) {
         TableOperations ops = ((HasTableOperations) originTable).operations();
         MetadataTableType type = MetadataTableType.from(canonicalized.name());
@@ -164,7 +166,7 @@ public class CachingCatalog implements Catalog {
       }
     }
 
-    return tableCache.get(canonicalized, catalog::loadTable);
+    return table;
   }
 
   @Override
diff --git 
a/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java 
b/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java
index cb7ca641ea..c562788fa3 100644
--- a/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java
+++ b/core/src/test/java/org/apache/iceberg/hadoop/TestCachingCatalog.java
@@ -32,6 +32,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.iceberg.BaseMetadataTable;
 import org.apache.iceberg.CachingCatalog;
 import org.apache.iceberg.CatalogProperties;
 import org.apache.iceberg.MetadataTableType;
@@ -41,6 +42,7 @@ import org.apache.iceberg.TestableCachingCatalog;
 import org.apache.iceberg.catalog.Catalog;
 import org.apache.iceberg.catalog.Namespace;
 import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.exceptions.NoSuchTableException;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
 import org.apache.iceberg.relocated.com.google.common.collect.Lists;
 import org.apache.iceberg.util.FakeTicker;
@@ -166,6 +168,43 @@ public class TestCachingCatalog extends 
HadoopTableTestBase {
         .isEqualTo("hadoop.db.ns1.ns2.tbl.snapshots");
   }
 
+  @Test
+  public void testNonExistingTable() throws Exception {
+    Catalog catalog = CachingCatalog.wrap(hadoopCatalog());
+
+    TableIdentifier tableIdent = TableIdentifier.of("otherDB", "otherTbl");
+
+    assertThatThrownBy(() -> catalog.loadTable(tableIdent))
+        .isInstanceOf(NoSuchTableException.class)
+        .hasMessage("Table does not exist: otherDB.otherTbl");
+  }
+
+  @Test
+  public void testTableWithMetadataTableName() throws Exception {
+    TestableCachingCatalog catalog =
+        TestableCachingCatalog.wrap(hadoopCatalog(), EXPIRATION_TTL, ticker);
+    TableIdentifier tableIdent = TableIdentifier.of("db", "ns1", "ns2", 
"partitions");
+    TableIdentifier metaTableIdent =
+        TableIdentifier.of("db", "ns1", "ns2", "partitions", "partitions");
+
+    catalog.createTable(tableIdent, SCHEMA, SPEC, ImmutableMap.of("key2", 
"value2"));
+    catalog.cache().invalidateAll();
+
+    Table table = catalog.loadTable(tableIdent);
+    assertThat(table.name()).isEqualTo("hadoop.db.ns1.ns2.partitions");
+    assertThat(catalog.cache().asMap()).containsKey(tableIdent);
+    assertThat(catalog.cache().asMap()).doesNotContainKey(metaTableIdent);
+
+    catalog.cache().invalidateAll();
+    assertThat(catalog.cache().asMap()).doesNotContainKey(tableIdent);
+
+    Table metaTable = catalog.loadTable(metaTableIdent);
+    assertThat(metaTable).isInstanceOf(BaseMetadataTable.class);
+    
assertThat(metaTable.name()).isEqualTo("hadoop.db.ns1.ns2.partitions.partitions");
+    assertThat(catalog.cache().asMap()).containsKey(tableIdent);
+    assertThat(catalog.cache().asMap()).containsKey(metaTableIdent);
+  }
+
   @Test
   public void testTableExpiresAfterInterval() throws IOException {
     TestableCachingCatalog catalog =