[ 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)