From 352bac194d8c4fbc4f07b8d14a6ab737fa712588 Mon Sep 17 00:00:00 2001 From: nk1506 Date: Tue, 17 Sep 2024 13:35:51 +0530 Subject: [PATCH] Added specific props for view, Added test for Hive Catalog --- .../apache/iceberg/BaseMetastoreCatalog.java | 52 ++++++++-------- .../org/apache/iceberg/CatalogProperties.java | 2 + .../view/BaseMetastoreViewCatalog.java | 11 +++- .../apache/iceberg/jdbc/TestJdbcCatalog.java | 26 ++++---- .../iceberg/hive/TestHiveViewCatalog.java | 61 +++++++++++++++++++ 5 files changed, 110 insertions(+), 42 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java b/core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java index 83ec1154fcfa..e794b3121dc3 100644 --- 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 tableDefaultProperties() { - Map 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 tableDefaultProperties() { + Map 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 tableOverrideProperties() { - Map 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 tableOverrideProperties() { + Map 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 index 339c59b45d1b..c17ed59b2674 100644 --- 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 index 076d475b5b67..0b408e730710 100644 --- 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 index f7f52f06adb0..6782410a255a 100644 --- 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 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 index 3c195e256520..8d16fd2efbf9 100644 --- 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 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 parameters = Maps.newHashMap(); parameters.put(