kasakrisz commented on code in PR #6449:
URL: https://github.com/apache/hive/pull/6449#discussion_r3154174350


##########
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/MetastoreUtil.java:
##########
@@ -148,6 +153,65 @@ public static Table toHiveTable(org.apache.iceberg.Table 
table, Configuration co
     return result;
   }
 
+  /**
+   * Builds a Hive metastore {@link Table} representation for an Iceberg 
{@link View}, for clients
+   * (e.g. {@code HiveRESTCatalogClient}) that bridge Iceberg catalog metadata 
into the HMS API.
+   */
+  public static Table toHiveView(View view, Configuration conf) {
+    Table result = new Table();
+    TableName tableName =
+        TableName.fromString(
+            view.name(), MetaStoreUtils.getDefaultCatalog(conf), 
Warehouse.DEFAULT_DATABASE_NAME);
+    result.setCatName(tableName.getCat());
+    result.setDbName(tableName.getDb());
+    result.setTableName(tableName.getTable());
+    result.setTableType(TableType.VIRTUAL_VIEW.toString());
+
+    ViewMetadata metadata = ((BaseView) view).operations().current();
+    String sqlText = viewSqlText(view, metadata);
+    result.setViewOriginalText(sqlText);
+    result.setViewExpandedText(sqlText);
+
+    long nowMillis = System.currentTimeMillis();
+    int nowSec = (int) (nowMillis / 1000);
+    String owner =
+        PropertyUtil.propertyAsString(
+            metadata.properties(), HiveCatalog.HMS_TABLE_OWNER, 
System.getProperty("user.name"));
+    result.setOwner(owner);
+    result.setCreateTime(nowSec);
+    result.setLastAccessTime(nowSec);
+    result.setRetention(Integer.MAX_VALUE);
+
+    boolean hiveEngineEnabled = false;

Review Comment:
   What is `hiveEngineEnabled` and why is it `false`?



##########
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/NativeIcebergViewSupport.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.hive;
+
+import java.io.Closeable;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.iceberg.CatalogUtil;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.catalog.ViewCatalog;
+import org.apache.iceberg.view.ViewBuilder;
+
+/**
+ * Commits a native Iceberg view through the configured default Iceberg 
catalog (HiveCatalog or REST
+ * catalog, etc.) when {@code Catalog} also implements {@link ViewCatalog}.
+ */
+public final class NativeIcebergViewSupport {
+
+  /** HMS parameter aligned with Hive's {@code 
CreateViewDesc#ICEBERG_NATIVE_VIEW_PROPERTY}. */
+  public static final String ICEBERG_NATIVE_VIEW_PROPERTY = 
"hive.iceberg.native.view";
+
+  private NativeIcebergViewSupport() {
+  }
+
+  /**
+   * Creates or replaces a view in the Iceberg catalog.
+   *
+   * @return {@code false} if skipped because {@code ifNotExists} is true and 
the view already exists
+   */
+  public static boolean createOrReplaceNativeView(Configuration conf, String 
databaseName, String viewName,
+      List<FieldSchema> fieldSchemas, String viewSql, Map<String, String> 
tblProperties, String comment,
+      boolean replace, boolean ifNotExists) throws Exception {
+
+    TableIdentifier identifier = TableIdentifier.of(databaseName, viewName);
+    String catalogName = IcebergCatalogProperties.getCatalogName(conf);
+    Map<String, String> catalogProps = 
IcebergCatalogProperties.getCatalogProperties(conf, catalogName);
+    Catalog catalog = CatalogUtil.buildIcebergCatalog(catalogName, 
catalogProps, conf);
+    try {
+      ViewCatalog viewCatalog = asViewCatalog(catalog, catalogName);
+      if (!replace && ifNotExists && viewCatalog.viewExists(identifier)) {
+        return false;
+      }
+
+      ViewBuilder builder = startViewBuilder(viewCatalog, identifier, 
fieldSchemas, viewSql);
+      builder = applyCommentAndTblProps(builder, tblProperties, comment);
+      commitView(builder, replace);
+      return true;
+    } finally {
+      if (catalog instanceof Closeable) {
+        ((Closeable) catalog).close();
+      }
+    }
+  }
+
+  private static ViewCatalog asViewCatalog(Catalog catalog, String 
catalogName) {
+    if (!(catalog instanceof ViewCatalog)) {
+      throw new UnsupportedOperationException(
+          String.format(
+                  "Iceberg catalog '%s' does not implement ViewCatalog.",
+                  catalogName) +
+              " Native views require a catalog that implements ViewCatalog 
(e.g. HiveCatalog or REST).");
+    }
+    return (ViewCatalog) catalog;
+  }
+
+  private static ViewBuilder startViewBuilder(

Review Comment:
   nit.: `startViewBuilder`, `applyCommentAndTblProps`, `commitView` doesn't 
add much value when we already have a builder.



##########
iceberg/iceberg-catalog/src/main/java/org/apache/iceberg/hive/NativeIcebergViewSupport.java:
##########
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.hive;
+
+import java.io.Closeable;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.iceberg.CatalogUtil;
+import org.apache.iceberg.catalog.Catalog;
+import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.catalog.ViewCatalog;
+import org.apache.iceberg.view.ViewBuilder;
+
+/**
+ * Commits a native Iceberg view through the configured default Iceberg 
catalog (HiveCatalog or REST
+ * catalog, etc.) when {@code Catalog} also implements {@link ViewCatalog}.
+ */
+public final class NativeIcebergViewSupport {
+
+  /** HMS parameter aligned with Hive's {@code 
CreateViewDesc#ICEBERG_NATIVE_VIEW_PROPERTY}. */
+  public static final String ICEBERG_NATIVE_VIEW_PROPERTY = 
"hive.iceberg.native.view";
+
+  private NativeIcebergViewSupport() {
+  }
+
+  /**
+   * Creates or replaces a view in the Iceberg catalog.
+   *
+   * @return {@code false} if skipped because {@code ifNotExists} is true and 
the view already exists
+   */
+  public static boolean createOrReplaceNativeView(Configuration conf, String 
databaseName, String viewName,
+      List<FieldSchema> fieldSchemas, String viewSql, Map<String, String> 
tblProperties, String comment,
+      boolean replace, boolean ifNotExists) throws Exception {
+
+    TableIdentifier identifier = TableIdentifier.of(databaseName, viewName);
+    String catalogName = IcebergCatalogProperties.getCatalogName(conf);
+    Map<String, String> catalogProps = 
IcebergCatalogProperties.getCatalogProperties(conf, catalogName);
+    Catalog catalog = CatalogUtil.buildIcebergCatalog(catalogName, 
catalogProps, conf);
+    try {
+      ViewCatalog viewCatalog = asViewCatalog(catalog, catalogName);
+      if (!replace && ifNotExists && viewCatalog.viewExists(identifier)) {
+        return false;
+      }
+
+      ViewBuilder builder = startViewBuilder(viewCatalog, identifier, 
fieldSchemas, viewSql);
+      builder = applyCommentAndTblProps(builder, tblProperties, comment);
+      commitView(builder, replace);
+      return true;
+    } finally {
+      if (catalog instanceof Closeable) {
+        ((Closeable) catalog).close();
+      }
+    }
+  }
+
+  private static ViewCatalog asViewCatalog(Catalog catalog, String 
catalogName) {
+    if (!(catalog instanceof ViewCatalog)) {
+      throw new UnsupportedOperationException(
+          String.format(
+                  "Iceberg catalog '%s' does not implement ViewCatalog.",
+                  catalogName) +
+              " Native views require a catalog that implements ViewCatalog 
(e.g. HiveCatalog or REST).");
+    }
+    return (ViewCatalog) catalog;
+  }
+
+  private static ViewBuilder startViewBuilder(
+      ViewCatalog viewCatalog,
+      TableIdentifier identifier,
+      List<FieldSchema> fieldSchemas,
+      String viewSql) {
+    return viewCatalog
+        .buildView(identifier)
+        .withSchema(HiveSchemaUtil.convert(fieldSchemas, 
Collections.emptyMap(), true))
+        .withDefaultNamespace(Namespace.of(identifier.namespace().level(0)))
+        .withQuery("hive", viewSql)
+        .withProperty(ICEBERG_NATIVE_VIEW_PROPERTY, "true");
+  }
+
+  private static ViewBuilder applyCommentAndTblProps(
+      ViewBuilder builder, Map<String, String> tblProperties, String comment) {
+    ViewBuilder viewBuilder = builder;
+    if (comment != null && !comment.isEmpty()) {

Review Comment:
   how about `isNotBlank` ?



##########
iceberg/iceberg-handler/src/test/queries/positive/iceberg_native_view.q:
##########
@@ -0,0 +1,38 @@
+-- SORT_QUERY_RESULTS
+
+create database ice_native_view_db;
+use ice_native_view_db;
+
+create table src_ice (
+    first_name string, 
+    last_name string
+ )
+partitioned by (dept_id bigint)
+stored by iceberg stored as orc;
+
+insert into src_ice values ('fn1','ln1', 1);
+insert into src_ice values ('fn2','ln2', 1);
+insert into src_ice values ('fn3','ln3', 1);
+insert into src_ice values ('fn4','ln4', 1);
+insert into src_ice values ('fn5','ln5', 2);
+insert into src_ice values ('fn6','ln6', 2);
+insert into src_ice values ('fn7','ln7', 2);
+
+update src_ice set last_name = 'ln1a' where first_name='fn1';
+update src_ice set last_name = 'ln2a' where first_name='fn2';
+update src_ice set last_name = 'ln3a' where first_name='fn3';
+update src_ice set last_name = 'ln4a' where first_name='fn4';
+update src_ice set last_name = 'ln5a' where first_name='fn5';
+update src_ice set last_name = 'ln6a' where first_name='fn6';
+update src_ice set last_name = 'ln7a' where first_name='fn7';
+
+delete from src_ice where last_name in ('ln1a', 'ln2a', 'ln7a');

Review Comment:
   Does the number of updates relevant?
   What is the purpose of these updates and deletes? No logical view exists at 
this point. 
   
   Also the way of logical views are used shouldn't depend on where the view 
metadata is stored.
   



##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewAnalyzer.java:
##########
@@ -75,6 +75,17 @@ public void analyzeInternal(ASTNode root) throws 
SemanticException {
     List<String> partitionColumnNames = 
children.containsKey(HiveParser.TOK_VIEWPARTCOLS) ?
         getColumnNames((ASTNode) 
children.remove(HiveParser.TOK_VIEWPARTCOLS).getChild(0)) : null;
 
+    ASTNode storageClause = null;
+    for (int storageTokenType : new int[] { HiveParser.TOK_STORAGEHANDLER, 
HiveParser.TOK_TABLEFILEFORMAT,
+        HiveParser.TOK_FILEFORMAT_GENERIC }) {
+      ASTNode storageClauseCandidate = children.remove(storageTokenType);
+      if (storageClauseCandidate != null) {
+        storageClause = storageClauseCandidate;
+        break;
+      }
+    }
+    boolean icebergNativeView = 
validateOptionalViewStorageClause(storageClause);

Review Comment:
   Please do not hardcode anything like `Iceberg` into compiler code. The 
compiler is independent from the storage handler. I'm aware that we already 
hove lots of code which violates this principal and it already causes lots of 
troubles.
   



##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java:
##########
@@ -34,26 +34,38 @@
 public class CreateViewDesc extends AbstractCreateViewDesc {
   private static final long serialVersionUID = 1L;
 
+  /** HMS table property set when the view is declared with {@code STORED BY 
ICEBERG} (native Iceberg view). */
+  public static final String ICEBERG_NATIVE_VIEW_PROPERTY = 
"hive.iceberg.native.view";

Review Comment:
   Please remove this from here.



##########
iceberg/iceberg-catalog/src/test/java/org/apache/iceberg/hive/TestNativeIcebergViewSupport.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.hive;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.iceberg.CatalogUtil;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.view.BaseView;
+import org.apache.iceberg.view.View;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import static org.apache.iceberg.CatalogUtil.ICEBERG_CATALOG_TYPE;
+import static org.apache.iceberg.CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE;
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class TestNativeIcebergViewSupport {
+
+  private static final String DB = "native_vw_db";
+  private static final String VIEW = "native_vw";
+
+  @RegisterExtension
+  private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION =
+      HiveMetastoreExtension.builder().withDatabase(DB).build();
+
+  @AfterEach
+  public void dropView() {
+    HiveCatalog cat = verifyCatalog();
+    TableIdentifier id = TableIdentifier.of(DB, VIEW);
+    if (cat.viewExists(id)) {
+      cat.dropView(id);
+    }
+  }
+
+  private HiveConf nativeViewConf() {
+    HiveConf conf = new HiveConf(HIVE_METASTORE_EXTENSION.hiveConf());
+    MetastoreConf.setVar(conf, MetastoreConf.ConfVars.CATALOG_DEFAULT, "hive");
+    conf.set(
+        IcebergCatalogProperties.catalogPropertyConfigKey("hive", 
ICEBERG_CATALOG_TYPE),
+        ICEBERG_CATALOG_TYPE_HIVE);
+    return conf;
+  }
+
+  private HiveCatalog verifyCatalog() {
+    return (HiveCatalog)
+        CatalogUtil.loadCatalog(
+            HiveCatalog.class.getName(),
+            ICEBERG_CATALOG_TYPE_HIVE,
+            ImmutableMap.of(
+                CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS,
+                String.valueOf(TimeUnit.SECONDS.toMillis(10))),
+            HIVE_METASTORE_EXTENSION.hiveConf());
+  }
+
+  @Test
+  public void testCreateCommitsNativeViewWithMarkerProperty() throws Exception 
{
+    HiveConf conf = nativeViewConf();
+    List<FieldSchema> cols =
+        Arrays.asList(new FieldSchema("id", "int", null), new 
FieldSchema("name", "string", null));
+    String sql = String.format("select id, name from %s.src_tbl", DB);
+    Map<String, String> props = Collections.singletonMap("k1", "v1");
+
+    boolean created =
+        NativeIcebergViewSupport.createOrReplaceNativeView(
+            conf, DB, VIEW, cols, sql, props, "hello-view", false, false);
+    assertThat(created).isTrue();
+
+    HiveCatalog cat = verifyCatalog();
+    TableIdentifier id = TableIdentifier.of(DB, VIEW);
+    assertThat(cat.viewExists(id)).isTrue();
+    View view = cat.loadView(id);
+    
assertThat(view.properties().get(NativeIcebergViewSupport.ICEBERG_NATIVE_VIEW_PROPERTY))
+        .isEqualTo("true");
+    assertThat(view.properties().get("comment")).isEqualTo("hello-view");
+    assertThat(view.properties().get("k1")).isEqualTo("v1");
+    HiveViewOperations ops = (HiveViewOperations) ((BaseView) 
view).operations();
+    assertThat(ops.current().currentVersion().representations()).isNotEmpty();
+  }
+
+  @Test
+  public void testIfNotExistsReturnsFalseWhenViewExists() throws Exception {

Review Comment:
   Isn't the test method name `testIfNotExistsReturnsFalseWhenViewExists` is 
misleading? We are testing `createOrReplaceNativeView` not the `IfNotExists` 
method.



##########
iceberg/iceberg-handler/src/test/queries/positive/iceberg_native_view.q:
##########
@@ -0,0 +1,38 @@
+-- SORT_QUERY_RESULTS
+
+create database ice_native_view_db;
+use ice_native_view_db;
+
+create table src_ice (
+    first_name string, 
+    last_name string
+ )
+partitioned by (dept_id bigint)
+stored by iceberg stored as orc;
+
+insert into src_ice values ('fn1','ln1', 1);
+insert into src_ice values ('fn2','ln2', 1);
+insert into src_ice values ('fn3','ln3', 1);
+insert into src_ice values ('fn4','ln4', 1);
+insert into src_ice values ('fn5','ln5', 2);
+insert into src_ice values ('fn6','ln6', 2);
+insert into src_ice values ('fn7','ln7', 2);

Review Comment:
   Does the number of insert statements relevant? Can we insert all the records 
in one insert statement? (It is faster)



##########
iceberg/iceberg-catalog/src/test/java/org/apache/iceberg/hive/TestNativeIcebergViewSupport.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.hive;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.iceberg.CatalogUtil;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.view.BaseView;
+import org.apache.iceberg.view.View;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import static org.apache.iceberg.CatalogUtil.ICEBERG_CATALOG_TYPE;
+import static org.apache.iceberg.CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE;
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class TestNativeIcebergViewSupport {
+
+  private static final String DB = "native_vw_db";
+  private static final String VIEW = "native_vw";
+
+  @RegisterExtension
+  private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION =
+      HiveMetastoreExtension.builder().withDatabase(DB).build();
+
+  @AfterEach
+  public void dropView() {
+    HiveCatalog cat = verifyCatalog();
+    TableIdentifier id = TableIdentifier.of(DB, VIEW);
+    if (cat.viewExists(id)) {
+      cat.dropView(id);

Review Comment:
   What is the result of  `dropView` when the view with the specified name 
doesn't exists? I thinkg about whether the `cat.viewExists(id)` is necessary



##########
iceberg/iceberg-catalog/src/test/java/org/apache/iceberg/hive/TestNativeIcebergViewSupport.java:
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iceberg.hive;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.metastore.api.FieldSchema;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.iceberg.CatalogUtil;
+import org.apache.iceberg.catalog.TableIdentifier;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.view.BaseView;
+import org.apache.iceberg.view.View;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import static org.apache.iceberg.CatalogUtil.ICEBERG_CATALOG_TYPE;
+import static org.apache.iceberg.CatalogUtil.ICEBERG_CATALOG_TYPE_HIVE;
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class TestNativeIcebergViewSupport {
+
+  private static final String DB = "native_vw_db";
+  private static final String VIEW = "native_vw";
+
+  @RegisterExtension
+  private static final HiveMetastoreExtension HIVE_METASTORE_EXTENSION =
+      HiveMetastoreExtension.builder().withDatabase(DB).build();
+
+  @AfterEach
+  public void dropView() {
+    HiveCatalog cat = verifyCatalog();
+    TableIdentifier id = TableIdentifier.of(DB, VIEW);
+    if (cat.viewExists(id)) {
+      cat.dropView(id);
+    }
+  }
+
+  private HiveConf nativeViewConf() {
+    HiveConf conf = new HiveConf(HIVE_METASTORE_EXTENSION.hiveConf());
+    MetastoreConf.setVar(conf, MetastoreConf.ConfVars.CATALOG_DEFAULT, "hive");
+    conf.set(
+        IcebergCatalogProperties.catalogPropertyConfigKey("hive", 
ICEBERG_CATALOG_TYPE),
+        ICEBERG_CATALOG_TYPE_HIVE);
+    return conf;
+  }
+
+  private HiveCatalog verifyCatalog() {
+    return (HiveCatalog)
+        CatalogUtil.loadCatalog(
+            HiveCatalog.class.getName(),
+            ICEBERG_CATALOG_TYPE_HIVE,
+            ImmutableMap.of(
+                CatalogProperties.CLIENT_POOL_CACHE_EVICTION_INTERVAL_MS,
+                String.valueOf(TimeUnit.SECONDS.toMillis(10))),
+            HIVE_METASTORE_EXTENSION.hiveConf());
+  }
+
+  @Test
+  public void testCreateCommitsNativeViewWithMarkerProperty() throws Exception 
{
+    HiveConf conf = nativeViewConf();
+    List<FieldSchema> cols =
+        Arrays.asList(new FieldSchema("id", "int", null), new 
FieldSchema("name", "string", null));
+    String sql = String.format("select id, name from %s.src_tbl", DB);
+    Map<String, String> props = Collections.singletonMap("k1", "v1");
+
+    boolean created =
+        NativeIcebergViewSupport.createOrReplaceNativeView(
+            conf, DB, VIEW, cols, sql, props, "hello-view", false, false);
+    assertThat(created).isTrue();
+
+    HiveCatalog cat = verifyCatalog();
+    TableIdentifier id = TableIdentifier.of(DB, VIEW);
+    assertThat(cat.viewExists(id)).isTrue();
+    View view = cat.loadView(id);
+    
assertThat(view.properties().get(NativeIcebergViewSupport.ICEBERG_NATIVE_VIEW_PROPERTY))
+        .isEqualTo("true");
+    assertThat(view.properties().get("comment")).isEqualTo("hello-view");
+    assertThat(view.properties().get("k1")).isEqualTo("v1");
+    HiveViewOperations ops = (HiveViewOperations) ((BaseView) 
view).operations();
+    assertThat(ops.current().currentVersion().representations()).isNotEmpty();
+  }
+
+  @Test
+  public void testIfNotExistsReturnsFalseWhenViewExists() throws Exception {
+    HiveConf conf = nativeViewConf();
+    List<FieldSchema> cols = Collections.singletonList(new FieldSchema("id", 
"int", null));
+
+    assertThat(
+            NativeIcebergViewSupport.createOrReplaceNativeView(
+                conf, DB, VIEW, cols, "select 1 as id", null, null, false, 
false))
+        .isTrue();
+    assertThat(
+            NativeIcebergViewSupport.createOrReplaceNativeView(
+                conf, DB, VIEW, cols, "select 2 as id", null, null, false, 
true))
+        .isFalse();
+  }
+
+  @Test
+  public void testReplaceUpdatesView() throws Exception {
+    HiveConf conf = nativeViewConf();
+    List<FieldSchema> cols = Collections.singletonList(new FieldSchema("id", 
"int", null));
+
+    NativeIcebergViewSupport.createOrReplaceNativeView(
+        conf, DB, VIEW, cols, "select 1 as id", null, null, false, false);
+    assertThat(
+            NativeIcebergViewSupport.createOrReplaceNativeView(
+                conf, DB, VIEW, cols, "select 2 as id", null, null, true, 
false))
+        .isTrue();
+
+    assertThat(verifyCatalog().viewExists(TableIdentifier.of(DB, 
VIEW))).isTrue();
+  }

Review Comment:
   What is the purpose of this test? Shouldn't the definition query change 
checked?



##########
iceberg/iceberg-handler/src/test/queries/positive/iceberg_native_view.q:
##########
@@ -0,0 +1,38 @@
+-- SORT_QUERY_RESULTS
+
+create database ice_native_view_db;
+use ice_native_view_db;
+
+create table src_ice (
+    first_name string, 
+    last_name string
+ )
+partitioned by (dept_id bigint)
+stored by iceberg stored as orc;
+
+insert into src_ice values ('fn1','ln1', 1);
+insert into src_ice values ('fn2','ln2', 1);
+insert into src_ice values ('fn3','ln3', 1);
+insert into src_ice values ('fn4','ln4', 1);
+insert into src_ice values ('fn5','ln5', 2);
+insert into src_ice values ('fn6','ln6', 2);
+insert into src_ice values ('fn7','ln7', 2);
+
+update src_ice set last_name = 'ln1a' where first_name='fn1';
+update src_ice set last_name = 'ln2a' where first_name='fn2';
+update src_ice set last_name = 'ln3a' where first_name='fn3';
+update src_ice set last_name = 'ln4a' where first_name='fn4';
+update src_ice set last_name = 'ln5a' where first_name='fn5';
+update src_ice set last_name = 'ln6a' where first_name='fn6';
+update src_ice set last_name = 'ln7a' where first_name='fn7';
+
+delete from src_ice where last_name in ('ln1a', 'ln2a', 'ln7a');
+
+create view v_ice as select * from src_ice stored by iceberg;
+
+select * from v_ice;
+

Review Comment:
   Could you please add 
   * logical view which does some transformation on it's base table and query 
from it?
   * create views when the schema is specified and not specified.



##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java:
##########
@@ -34,26 +34,38 @@
 public class CreateViewDesc extends AbstractCreateViewDesc {
   private static final long serialVersionUID = 1L;
 
+  /** HMS table property set when the view is declared with {@code STORED BY 
ICEBERG} (native Iceberg view). */
+  public static final String ICEBERG_NATIVE_VIEW_PROPERTY = 
"hive.iceberg.native.view";
+
   private final String comment;
   private final Map<String, String> properties;
   private final List<String> partitionColumnNames;
   private final boolean ifNotExists;
   private final boolean replace;
   private final List<FieldSchema> partitionColumns;
+  private final boolean icebergNativeView;

Review Comment:
   Please remove this from here.



##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/view/create/CreateViewDesc.java:
##########
@@ -89,6 +101,11 @@ public boolean isReplace() {
     return replace;
   }
 
+  @Explain(displayName = "iceberg native view", displayOnlyOnTrue = true)
+  public boolean isIcebergNativeView() {
+    return icebergNativeView;
+  }

Review Comment:
   Please remove this from here.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to