[ 
https://issues.apache.org/jira/browse/HIVE-26847?focusedWorklogId=833544&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-833544
 ]

ASF GitHub Bot logged work on HIVE-26847:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 14/Dec/22 18:46
            Start Date: 14/Dec/22 18:46
    Worklog Time Spent: 10m 
      Work Description: cnauroth commented on code in PR #3856:
URL: https://github.com/apache/hive/pull/3856#discussion_r1048838725


##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMaterialisedViewDisabled.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.SourceTable;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder;
+import org.apache.hadoop.hive.metastore.client.builder.TableBuilder;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.security.HadoopThriftAuthBridge;
+import org.apache.thrift.TException;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+@Category(MetastoreUnitTest.class)
+public class TestMaterialisedViewDisabled {
+  private Configuration conf;
+  private HiveMetaStoreClient client;
+
+  @Before
+  public void setUp() throws Exception {
+    conf = MetastoreConf.newMetastoreConf();
+    MetastoreConf.setBoolVar(conf, 
MetastoreConf.ConfVars.MATERIALIZED_VIEW_ENABLED, false);
+    MetaStoreTestUtils.setConfForStandloneMode(conf);
+    int port = 
MetaStoreTestUtils.startMetaStoreWithRetry(HadoopThriftAuthBridge.getBridge(), 
conf);
+    MetastoreConf.setVar(conf, MetastoreConf.ConfVars.THRIFT_URIS, 
"thrift://localhost:" + port);
+    MetastoreConf.setBoolVar(conf, MetastoreConf.ConfVars.EXECUTE_SET_UGI, 
false);
+    client = new HiveMetaStoreClient(conf);

Review Comment:
   Could you please add an `@After` method that calls `client.close();`?



##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:
##########
@@ -1127,6 +1127,8 @@ public enum ConfVars {
       "command should handle partition retention. If enabled, and if a 
specific partition's age exceeded\n" +
       "retention period the partition will be dropped along with data"),
 
+    MATERIALIZED_VIEW_ENABLED("metastore.materialized.view.enabled", 
"metastore.materialized.view.enabled",true, "if " +

Review Comment:
   Very minor nitpick: please add a space before `true` and capitalize `"If "`.



##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMaterialisedViewDisabled.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.SourceTable;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder;
+import org.apache.hadoop.hive.metastore.client.builder.TableBuilder;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.security.HadoopThriftAuthBridge;
+import org.apache.thrift.TException;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+@Category(MetastoreUnitTest.class)
+public class TestMaterialisedViewDisabled {
+  private Configuration conf;
+  private HiveMetaStoreClient client;
+
+  @Before
+  public void setUp() throws Exception {
+    conf = MetastoreConf.newMetastoreConf();
+    MetastoreConf.setBoolVar(conf, 
MetastoreConf.ConfVars.MATERIALIZED_VIEW_ENABLED, false);
+    MetaStoreTestUtils.setConfForStandloneMode(conf);
+    int port = 
MetaStoreTestUtils.startMetaStoreWithRetry(HadoopThriftAuthBridge.getBridge(), 
conf);
+    MetastoreConf.setVar(conf, MetastoreConf.ConfVars.THRIFT_URIS, 
"thrift://localhost:" + port);
+    MetastoreConf.setBoolVar(conf, MetastoreConf.ConfVars.EXECUTE_SET_UGI, 
false);
+    client = new HiveMetaStoreClient(conf);
+  }
+
+  @Test
+  public void testMaterialisedViewDisabled() throws TException {
+    String dbName = "db";
+    List<String> tableNames = Arrays.asList("table1", "table2", "table3", 
"table4", "table5");
+
+    Set<Table> tablesUsed = new HashSet<>();
+    new DatabaseBuilder().setName(dbName).create(client, conf);
+    for (String tableName : tableNames) {
+      tablesUsed.add(createTable(dbName, tableName));
+    }
+
+    Assert.assertThrows("Creation of MATERIALIZED_VIEW is disabled via 
metastore.materialized.view.enabled",
+        MetaException.class, () -> createMaterializedView(dbName, "mv1", 
tablesUsed));
+  }
+
+   Table createTable(String dbName, String tableName) throws TException {
+    return new TableBuilder()
+        .setDbName(dbName)
+        .setTableName(tableName)
+        .addCol("foo", "string")
+        .addCol("bar", "string")
+        .create(client, conf);
+  }
+
+  private void createMaterializedView(String dbName, String tableName, 
Set<Table> tablesUsed)
+      throws TException {
+    Set<SourceTable> sourceTables = new HashSet<>(tablesUsed.size());
+    for (Table table : tablesUsed) {
+      sourceTables.add(createSourceTable(table));
+    }
+
+    new TableBuilder().setDbName(dbName)
+        .setTableName(tableName)
+        .setType(TableType.MATERIALIZED_VIEW.name())
+        .addMaterializedViewReferencedTables(sourceTables)
+        .addCol("foo", "string")
+        .addCol("bar", "string")
+        .create(client, conf);
+  }
+
+  public static SourceTable createSourceTable(Table table) {

Review Comment:
   Consider marking this `private`.



##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMaterialisedViewDisabled.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.SourceTable;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder;
+import org.apache.hadoop.hive.metastore.client.builder.TableBuilder;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.security.HadoopThriftAuthBridge;
+import org.apache.thrift.TException;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+@Category(MetastoreUnitTest.class)
+public class TestMaterialisedViewDisabled {
+  private Configuration conf;
+  private HiveMetaStoreClient client;
+
+  @Before
+  public void setUp() throws Exception {
+    conf = MetastoreConf.newMetastoreConf();
+    MetastoreConf.setBoolVar(conf, 
MetastoreConf.ConfVars.MATERIALIZED_VIEW_ENABLED, false);
+    MetaStoreTestUtils.setConfForStandloneMode(conf);
+    int port = 
MetaStoreTestUtils.startMetaStoreWithRetry(HadoopThriftAuthBridge.getBridge(), 
conf);
+    MetastoreConf.setVar(conf, MetastoreConf.ConfVars.THRIFT_URIS, 
"thrift://localhost:" + port);
+    MetastoreConf.setBoolVar(conf, MetastoreConf.ConfVars.EXECUTE_SET_UGI, 
false);
+    client = new HiveMetaStoreClient(conf);
+  }
+
+  @Test
+  public void testMaterialisedViewDisabled() throws TException {
+    String dbName = "db";
+    List<String> tableNames = Arrays.asList("table1", "table2", "table3", 
"table4", "table5");
+
+    Set<Table> tablesUsed = new HashSet<>();
+    new DatabaseBuilder().setName(dbName).create(client, conf);
+    for (String tableName : tableNames) {
+      tablesUsed.add(createTable(dbName, tableName));
+    }
+
+    Assert.assertThrows("Creation of MATERIALIZED_VIEW is disabled via 
metastore.materialized.view.enabled",
+        MetaException.class, () -> createMaterializedView(dbName, "mv1", 
tablesUsed));
+  }
+
+   Table createTable(String dbName, String tableName) throws TException {

Review Comment:
   Consider marking this `private`.



##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestMaterialisedViewDisabled.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreUnitTest;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.SourceTable;
+import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder;
+import org.apache.hadoop.hive.metastore.client.builder.TableBuilder;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.security.HadoopThriftAuthBridge;
+import org.apache.thrift.TException;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+@Category(MetastoreUnitTest.class)
+public class TestMaterialisedViewDisabled {

Review Comment:
   There is a minor typo in the class name. Could you please make it 
"Materialized" ('z' instead of 's')?





Issue Time Tracking
-------------------

    Worklog Id:     (was: 833544)
    Time Spent: 0.5h  (was: 20m)

> Allow a config to disable creation of Materialized Views
> --------------------------------------------------------
>
>                 Key: HIVE-26847
>                 URL: https://issues.apache.org/jira/browse/HIVE-26847
>             Project: Hive
>          Issue Type: Improvement
>            Reporter: Ayush Saxena
>            Assignee: Ayush Saxena
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to