Added specific props for view, Added test for Hive Catalog

apache · Sep 17, 2024 · 352bac1 · 352bac1
1 parent e05731b
commit 352bac1
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 42 deletions.
diff --git a/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java b/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
@@ -254,34 +254,34 @@ private Transaction newReplaceTableTransaction(boolean orCreate) {
         return Transactions.replaceTableTransaction(identifier.toString(), ops, metadata);
       }
     }
-  }
 
-  /**
-   * Get default table properties set at Catalog level through catalog properties.
-   *
-   * @return default table properties specified in catalog properties
-   */
-  protected Map<String, String> tableDefaultProperties() {
-    Map<String, String> tableDefaultProperties =
-        PropertyUtil.propertiesWithPrefix(properties(), CatalogProperties.TABLE_DEFAULT_PREFIX);
-    LOG.info(
-        "Table properties set at catalog level through catalog properties: {}",
-        tableDefaultProperties);
-    return tableDefaultProperties;
-  }
+    /**
+     * Get default table properties set at Catalog level through catalog properties.
+     *
+     * @return default table properties specified in catalog properties
+     */
+    private Map<String, String> tableDefaultProperties() {
+      Map<String, String> tableDefaultProperties =
+          PropertyUtil.propertiesWithPrefix(properties(), CatalogProperties.TABLE_DEFAULT_PREFIX);
+      LOG.info(
+          "Table properties set at catalog level through catalog properties: {}",
+          tableDefaultProperties);
+      return tableDefaultProperties;
+    }
 
-  /**
-   * Get table properties that are enforced at Catalog level through catalog properties.
-   *
-   * @return default table properties enforced through catalog properties
-   */
-  protected Map<String, String> tableOverrideProperties() {
-    Map<String, String> tableOverrideProperties =
-        PropertyUtil.propertiesWithPrefix(properties(), CatalogProperties.TABLE_OVERRIDE_PREFIX);
-    LOG.info(
-        "Table properties enforced at catalog level through catalog properties: {}",
-        tableOverrideProperties);
-    return tableOverrideProperties;
+    /**
+     * Get table properties that are enforced at Catalog level through catalog properties.
+     *
+     * @return default table properties enforced through catalog properties
+     */
+    private Map<String, String> tableOverrideProperties() {
+      Map<String, String> tableOverrideProperties =
+          PropertyUtil.propertiesWithPrefix(properties(), CatalogProperties.TABLE_OVERRIDE_PREFIX);
+      LOG.info(
+          "Table properties enforced at catalog level through catalog properties: {}",
+          tableOverrideProperties);
+      return tableOverrideProperties;
+    }
   }
 
   protected static String fullTableName(String catalogName, TableIdentifier identifier) {

diff --git a/core/src/main/java/org/apache/iceberg/CatalogProperties.java b/core/src/main/java/org/apache/iceberg/CatalogProperties.java
@@ -29,6 +29,8 @@ private CatalogProperties() {}
   public static final String WAREHOUSE_LOCATION = "warehouse";
   public static final String TABLE_DEFAULT_PREFIX = "table-default.";
   public static final String TABLE_OVERRIDE_PREFIX = "table-override.";
+  public static final String VIEW_DEFAULT_PREFIX = "view-default.";
+  public static final String VIEW_OVERRIDE_PREFIX = "view-override.";
   public static final String METRICS_REPORTER_IMPL = "metrics-reporter-impl";
 
   /**

diff --git a/core/src/main/java/org/apache/iceberg/view/BaseMetastoreViewCatalog.java b/core/src/main/java/org/apache/iceberg/view/BaseMetastoreViewCatalog.java
@@ -21,6 +21,7 @@
 import java.util.List;
 import java.util.Map;
 import org.apache.iceberg.BaseMetastoreCatalog;
+import org.apache.iceberg.CatalogProperties;
 import org.apache.iceberg.EnvironmentContext;
 import org.apache.iceberg.Schema;
 import org.apache.iceberg.Transaction;
@@ -33,6 +34,7 @@
 import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
 import org.apache.iceberg.relocated.com.google.common.collect.Lists;
 import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.util.PropertyUtil;
 
 public abstract class BaseMetastoreViewCatalog extends BaseMetastoreCatalog implements ViewCatalog {
   protected abstract ViewOperations newViewOps(TableIdentifier identifier);
@@ -79,7 +81,8 @@ protected BaseViewBuilder(TableIdentifier identifier) {
       Preconditions.checkArgument(
           isValidIdentifier(identifier), "Invalid view identifier: %s", identifier);
       this.identifier = identifier;
-      this.properties.putAll(tableDefaultProperties());
+      this.properties.putAll(
+          PropertyUtil.propertiesWithPrefix(properties(), CatalogProperties.VIEW_DEFAULT_PREFIX));
     }
 
     @Override
@@ -156,7 +159,8 @@ private View create(ViewOperations ops) {
       Preconditions.checkState(
           null != defaultNamespace, "Cannot create view without specifying a default namespace");
 
-      properties.putAll(tableOverrideProperties());
+      properties.putAll(
+          PropertyUtil.propertiesWithPrefix(properties(), CatalogProperties.VIEW_OVERRIDE_PREFIX));
       ViewVersion viewVersion =
           ImmutableViewVersion.builder()
               .versionId(1)
@@ -192,7 +196,8 @@ private View replace(ViewOperations ops) {
       if (null == ops.current()) {
         throw new NoSuchViewException("View does not exist: %s", identifier);
       }
-      properties.putAll(tableOverrideProperties());
+      properties.putAll(
+          PropertyUtil.propertiesWithPrefix(properties(), CatalogProperties.VIEW_OVERRIDE_PREFIX));
 
       Preconditions.checkState(
           !representations.isEmpty(), "Cannot replace view without specifying a query");

diff --git a/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java b/core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java
@@ -1186,7 +1186,7 @@ public void testTablePropsDefinedAtCatalogLevel() throws IOException {
 
   @Test
   public void testViewPropsDefinedAtCatalogLevel() throws IOException {
-    TableIdentifier viewIdent = TableIdentifier.of("db", "ns1");
+    TableIdentifier identifier = TableIdentifier.of("db", "ns1");
     ImmutableMap<String, String> catalogProps =
         ImmutableMap.of(
             CatalogProperties.WAREHOUSE_LOCATION,
@@ -1195,22 +1195,22 @@ public void testViewPropsDefinedAtCatalogLevel() throws IOException {
             "jdbc:sqlite:file::memory:?icebergDBV0",
             JdbcUtil.SCHEMA_VERSION_PROPERTY,
             JdbcUtil.SchemaVersion.V1.name(),
-            "table-default.key1",
+            "view-default.key1",
             "catalog-default-key1",
-            "table-default.key2",
+            "view-default.key2",
             "catalog-default-key2",
-            "table-default.key3",
+            "view-default.key3",
             "catalog-default-key3",
-            "table-override.key3",
+            "view-override.key3",
             "catalog-override-key3",
-            "table-override.key4",
+            "view-override.key4",
             "catalog-override-key4");
     JdbcCatalog jdbcCatalog = new JdbcCatalog();
     jdbcCatalog.setConf(conf);
     jdbcCatalog.initialize("v0catalog", catalogProps);
     View view =
         jdbcCatalog
-            .buildView(viewIdent)
+            .buildView(identifier)
             .withQuery("spark", "SELECT * FROM t1")
             .withSchema(SCHEMA)
             .withDefaultNamespace(Namespace.of("db"))
@@ -1220,22 +1220,22 @@ public void testViewPropsDefinedAtCatalogLevel() throws IOException {
             .create();
 
     assertThat(view.properties().get("key1"))
-        .as("Table defaults set for the catalog must be added to the view properties.")
+        .as("View defaults set for the catalog must be added to the view properties.")
         .isEqualTo("catalog-default-key1");
     assertThat(view.properties().get("key2"))
-        .as("View property must override table default properties set at catalog level.")
+        .as("View property must override view default properties set at catalog level.")
         .isEqualTo("view-key2");
     assertThat(view.properties().get("key3"))
         .as(
-            "View property override set at catalog level must override table default"
-                + " properties set at catalog level and table property specified.")
+            "View property override set at catalog level must override view default"
+                + " properties set at catalog level and view property specified.")
         .isEqualTo("catalog-override-key3");
     assertThat(view.properties().get("key4"))
-        .as("Table override not in table props or defaults should be added to view properties")
+        .as("Table override not in view props or defaults should be added to view properties")
         .isEqualTo("catalog-override-key4");
     assertThat(view.properties().get("key5"))
         .as(
-            "Table properties without any catalog level default or override should be added to view"
+            "View properties without any catalog level default or override should be added to view"
                 + " properties.")
         .isEqualTo("view-key5");
   }

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
@@ -42,6 +42,7 @@
 import org.apache.iceberg.relocated.com.google.common.collect.Lists;
 import org.apache.iceberg.relocated.com.google.common.collect.Maps;
 import org.apache.iceberg.view.BaseView;
+import org.apache.iceberg.view.View;
 import org.apache.iceberg.view.ViewCatalogTests;
 import org.apache.thrift.TException;
 import org.junit.jupiter.api.AfterEach;
@@ -280,6 +281,66 @@ public void dropViewShouldDropMetadataFileIfGcEnabled() throws IOException {
     assertThat(catalog.viewExists(identifier)).isFalse();
   }
 
+  @Test
+  public void testViewPropsDefinedAtCatalogLevel() {
+    TableIdentifier identifier = TableIdentifier.of("db", "ns1");
+    if (requiresNamespaceCreate()) {
+      catalog.createNamespace(identifier.namespace());
+    }
+
+    ImmutableMap<String, String> catalogProps =
+        ImmutableMap.of(
+            CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS,
+            String.valueOf(TimeUnit.SECONDS.toMillis(10)),
+            "view-default.key1",
+            "catalog-default-key1",
+            "view-default.key2",
+            "catalog-default-key2",
+            "view-default.key3",
+            "catalog-default-key3",
+            "view-override.key3",
+            "catalog-override-key3",
+            "view-override.key4",
+            "catalog-override-key4");
+    HiveCatalog hiveCatalog =
+        (HiveCatalog)
+            CatalogUtil.loadCatalog(
+                HiveCatalog.class.getName(),
+                CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE,
+                catalogProps,
+                HIVE_METASTORE_EXTENSION.hiveConf());
+    View view =
+        hiveCatalog
+            .buildView(identifier)
+            .withQuery("spark", "SELECT * FROM t1")
+            .withSchema(SCHEMA)
+            .withDefaultNamespace(Namespace.of("db"))
+            .withProperty("key2", "view-key2")
+            .withProperty("key3", "view-key3")
+            .withProperty("key5", "view-key5")
+            .create();
+
+    assertThat(view.properties().get("key1"))
+        .as("View defaults set for the catalog must be added to the view properties.")
+        .isEqualTo("catalog-default-key1");
+    assertThat(view.properties().get("key2"))
+        .as("View property must override view default properties set at catalog level.")
+        .isEqualTo("view-key2");
+    assertThat(view.properties().get("key3"))
+        .as(
+            "View property override set at catalog level must override view default"
+                + " properties set at catalog level and view property specified.")
+        .isEqualTo("catalog-override-key3");
+    assertThat(view.properties().get("key4"))
+        .as("Table override not in view props or defaults should be added to view properties")
+        .isEqualTo("catalog-override-key4");
+    assertThat(view.properties().get("key5"))
+        .as(
+            "View properties without any catalog level default or override should be added to view"
+                + " properties.")
+        .isEqualTo("view-key5");
+  }
+
   private Table createHiveView(String hiveViewName, String dbName, String location) {
     Map<String, String> parameters = Maps.newHashMap();
     parameters.put(