diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java index a5c1461010ca..708a37632326 100644 --- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java +++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java @@ -786,6 +786,10 @@ public static enum ConfVars { "Name of the identifier factory to use when generating table/column names etc. \n" + "'datanucleus1' is used for backward compatibility with DataNucleus v1"), METASTORE_USE_LEGACY_VALUE_STRATEGY("datanucleus.rdbms.useLegacyNativeValueStrategy", true, ""), + METASTORE_QUERY_SQL_ALLOWALL("datanucleus.query.sql.allowAll", true, + "In strict JDO all SQL queries must begin with \"SELECT ...\", and consequently it " + + "is not possible to execute queries that change data. This DataNucleus property when set to true allows " + + "insert, update and delete operations from JDO SQL. Default value is true."), METASTORE_PLUGIN_REGISTRY_BUNDLE_CHECK("datanucleus.plugin.pluginRegistryBundleCheck", "LOG", "Defines what happens when plugin bundles are found and are duplicated [EXCEPTION|LOG|NONE]"), METASTORE_BATCH_RETRIEVE_MAX("hive.metastore.batch.retrieve.max", 300, diff --git a/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java b/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java index a5cc94e846c7..99e6e79633d2 100644 --- a/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java +++ b/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java @@ -928,4 +928,10 @@ public void addPrimaryKeys(List pks) public void addForeignKeys(List fks) throws InvalidObjectException, MetaException { } + + @Override + public long updateParameterWithExpectedValue(Table table, String key, String expectedValue, String newValue) + throws MetaException, NoSuchObjectException { + throw new UnsupportedOperationException("This Store doesn't support updating table parameter with expected value"); + } } \ No newline at end of file diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java index f5f6be5849f1..64ce269157c8 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java @@ -51,7 +51,6 @@ import org.apache.hadoop.ipc.RemoteException; import org.apache.hive.common.util.HiveStringUtils; -import javax.jdo.Constants; import java.io.IOException; import java.net.URI; import java.util.ArrayList; @@ -132,12 +131,7 @@ public void alterTable(RawStore msdb, Warehouse wh, String dbname, String expectedValue = environmentContext != null && environmentContext.getProperties() != null ? environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE) : null; - if (expectedKey != null) { - // If we have to check the expected state of the table we have to prevent nonrepeatable reads. - msdb.openTransaction(Constants.TX_REPEATABLE_READ); - } else { - msdb.openTransaction(); - } + msdb.openTransaction(); name = name.toLowerCase(); dbname = dbname.toLowerCase(); @@ -158,10 +152,20 @@ public void alterTable(RawStore msdb, Warehouse wh, String dbname, throw new InvalidOperationException("table " + dbname + "." + name + " doesn't exist"); } - if (expectedKey != null && expectedValue != null - && !expectedValue.equals(oldt.getParameters().get(expectedKey))) { - throw new MetaException("The table has been modified. The parameter value for key '" + expectedKey + "' is '" - + oldt.getParameters().get(expectedKey) + "'. The expected was value was '" + expectedValue + "'"); + if (expectedKey != null && expectedValue != null) { + String newValue = newt.getParameters().get(expectedKey); + if (newValue == null) { + throw new MetaException(String.format("New value for expected key %s is not set", expectedKey)); + } + if (!expectedValue.equals(oldt.getParameters().get(expectedKey))) { + throw new MetaException("The table has been modified. The parameter value for key '" + expectedKey + "' is '" + + oldt.getParameters().get(expectedKey) + "'. The expected was value was '" + expectedValue + "'"); + } + long affectedRows = msdb.updateParameterWithExpectedValue(oldt, expectedKey, expectedValue, newValue); + if (affectedRows != 1) { + // make sure concurrent modification exception messages have the same prefix + throw new MetaException("The table has been modified. The parameter value for key '" + expectedKey + "' is different"); + } } // Views derive the column type from the base table definition. So the view definition diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java b/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java index 48fa50d1bb20..678fce86c4a9 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java @@ -1964,4 +1964,13 @@ public List getPrimaryKeys(String db_name, String tbl_name) throw } return ret; } + + long updateTableParam(Table table, String key, String expectedValue, String newValue) { + String queryText = String.format("UPDATE \"TABLE_PARAMS\" SET \"PARAM_VALUE\" = ? " + + "WHERE \"PARAM_KEY\" = ? AND \"PARAM_VALUE\" = ? AND \"TBL_ID\" IN " + + "(SELECT \"TBL_ID\" FROM \"TBLS\" JOIN \"DBS\" ON \"TBLS\".\"DB_ID\" = \"DBS\".\"DB_ID\" WHERE \"TBL_NAME\" = '%s' AND \"NAME\" = '%s')", + table.getTableName(), table.getDbName()); + Query query = pm.newQuery("javax.jdo.query.SQL", queryText); + return (long) query.executeWithArray(newValue, key, expectedValue); + } } diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java index c719755b92c2..34221eaaab9f 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java @@ -608,6 +608,33 @@ public boolean openTransaction(String isolationLevel) { return result; } + @Override + public long updateParameterWithExpectedValue(Table table, String key, String expectedValue, String newValue) + throws MetaException, NoSuchObjectException { + final Table _table = table; + final String _key = key; + final String _expectedValue = expectedValue; + final String _newValue = newValue; + return new GetHelper(table.getDbName(), table.getTableName(), true, false) { + + @Override + protected String describeResult() { + return "Affected rows"; + } + + @Override + protected Long getSqlResult(GetHelper ctx) throws MetaException { + return directSql.updateTableParam(_table, _key, _expectedValue, _newValue); + } + + @Override + protected Long getJdoResult(GetHelper ctx) throws MetaException, NoSuchObjectException { + throw new UnsupportedOperationException( + "Cannot update parameter with JDO, make sure direct SQL is enabled"); + } + }.run(false); + } + /** * if this is the commit of the first open call then an actual commit is * called. diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java index 3e50db451f95..ecac7d557ba0 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java @@ -711,4 +711,12 @@ void createTableWithConstraints(Table tbl, List primaryKeys, void addPrimaryKeys(List pks) throws InvalidObjectException, MetaException; void addForeignKeys(List fks) throws InvalidObjectException, MetaException; + + /** + * Updates a given table parameter with expected value. + * + * @return the number of rows updated + */ + long updateParameterWithExpectedValue(Table table, String key, String expectedValue, String newValue) + throws MetaException, NoSuchObjectException; } diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java index 6e286d58c803..15e4e6b13ab3 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java @@ -2851,4 +2851,10 @@ public void addForeignKeys(List fks) throws InvalidObjectExceptio commitOrRoleBack(commit); } } + + @Override + public long updateParameterWithExpectedValue(Table table, String key, String expectedValue, String newValue) + throws MetaException, NoSuchObjectException { + throw new UnsupportedOperationException("This Store doesn't support updating table parameter with expected value"); + } } diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java b/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java index ec3b2376dd74..b66019da24f8 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java @@ -885,4 +885,10 @@ public void addForeignKeys(List fks) throws InvalidObjectException, MetaException { // TODO Auto-generated method stub } + + @Override + public long updateParameterWithExpectedValue(Table table, String key, String expectedValue, String newValue) + throws MetaException, NoSuchObjectException { + throw new UnsupportedOperationException("This Store doesn't support updating table parameter with expected value"); + } } diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java b/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java index a0f071a042b2..41b4456e9ce7 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java @@ -902,6 +902,12 @@ public void addForeignKeys(List fks) throws InvalidObjectException, MetaException { // TODO Auto-generated method stub } + + @Override + public long updateParameterWithExpectedValue(Table table, String key, String expectedValue, String newValue) + throws MetaException, NoSuchObjectException { + throw new UnsupportedOperationException("This Store doesn't support updating table parameter with expected value"); + } } diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java b/metastore/src/test/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java index 978df5e41426..fbfb23ffcf37 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hive.metastore.client; +import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hadoop.hive.metastore.IMetaStoreClient; import org.apache.hadoop.hive.metastore.RawStore; import org.apache.hadoop.hive.metastore.Warehouse; @@ -29,6 +30,7 @@ import org.apache.hadoop.hive.metastore.minihms.AbstractMetaStoreService; import org.junit.After; import org.junit.AfterClass; +import org.junit.Assume; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -136,6 +138,8 @@ public void tearDown() throws Exception { @Test public void testAlterTableExpectedPropertyMatch() throws Exception { + Assume.assumeTrue(HiveConf.getBoolVar(metaStore.getConf(), HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL)); + Assume.assumeTrue(HiveConf.getBoolVar(metaStore.getConf(), HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL_DDL)); Table originalTable = testTables[0]; EnvironmentContext context = new EnvironmentContext(); @@ -149,6 +153,8 @@ public void testAlterTableExpectedPropertyMatch() throws Exception { @Test(expected = MetaException.class) public void testAlterTableExpectedPropertyDifferent() throws Exception { + Assume.assumeTrue(HiveConf.getBoolVar(metaStore.getConf(), HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL)); + Assume.assumeTrue(HiveConf.getBoolVar(metaStore.getConf(), HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL_DDL)); Table originalTable = testTables[0]; EnvironmentContext context = new EnvironmentContext(); @@ -168,6 +174,8 @@ public void testAlterTableExpectedPropertyDifferent() throws Exception { */ @Test public void testAlterTableExpectedPropertyConcurrent() throws Exception { + Assume.assumeTrue(HiveConf.getBoolVar(metaStore.getConf(), HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL)); + Assume.assumeTrue(HiveConf.getBoolVar(metaStore.getConf(), HiveConf.ConfVars.METASTORE_TRY_DIRECT_SQL_DDL)); Table originalTable = testTables[0]; originalTable.getParameters().put("snapshot", "0"); diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java b/metastore/src/test/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java index 9b4fd704e969..de3e565da282 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hive.metastore.minihms; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; @@ -150,4 +151,8 @@ public void cleanWarehouseDirs() throws MetaException { */ public void stop() { } + + public Configuration getConf() { + return configuration; + } }