This is an automated email from the ASF dual-hosted git repository.
sunchao pushed a commit to branch branch-2.3
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/branch-2.3 by this push:
new 10b3b0310d4 HIVE-28121: (2.3) Use direct SQL for transactional
altering table parameter (#5204)
10b3b0310d4 is described below
commit 10b3b0310d486fdb665de9b0d70dfa6c5163be79
Author: Cheng Pan <cheng...@apache.org>
AuthorDate: Tue Apr 30 13:55:31 2024 +0800
HIVE-28121: (2.3) Use direct SQL for transactional altering table parameter
(#5204)
---
.../java/org/apache/hadoop/hive/conf/HiveConf.java | 4 ++++
.../hcatalog/listener/DummyRawStoreFailEvent.java | 6 +++++
.../hadoop/hive/metastore/HiveAlterHandler.java | 26 ++++++++++++---------
.../hadoop/hive/metastore/MetaStoreDirectSql.java | 9 ++++++++
.../apache/hadoop/hive/metastore/ObjectStore.java | 27 ++++++++++++++++++++++
.../org/apache/hadoop/hive/metastore/RawStore.java | 8 +++++++
.../hadoop/hive/metastore/hbase/HBaseStore.java | 6 +++++
.../metastore/DummyRawStoreControlledCommit.java | 6 +++++
.../metastore/DummyRawStoreForJdoConnection.java | 6 +++++
.../client/TestTablesCreateDropAlterTruncate.java | 8 +++++++
.../minihms/AbstractMetaStoreService.java | 5 ++++
11 files changed, 100 insertions(+), 11 deletions(-)
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 a5c1461010c..708a3763232 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 class HiveConf extends Configuration {
"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 a5cc94e846c..99e6e79633d 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 class DummyRawStoreFailEvent implements RawStore,
Configurable {
public void addForeignKeys(List<SQLForeignKey> 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 f5f6be5849f..64ce269157c 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.hive.metastore.api.hive_metastoreConstants;
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 class HiveAlterHandler implements AlterHandler {
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 class HiveAlterHandler implements AlterHandler {
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 48fa50d1bb2..678fce86c4a 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 @@ class MetaStoreDirectSql {
}
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 c719755b92c..34221eaaab9 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 class ObjectStore implements RawStore, Configurable
{
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<Long>(table.getDbName(), table.getTableName(), true,
false) {
+
+ @Override
+ protected String describeResult() {
+ return "Affected rows";
+ }
+
+ @Override
+ protected Long getSqlResult(GetHelper<Long> ctx) throws MetaException {
+ return directSql.updateTableParam(_table, _key, _expectedValue,
_newValue);
+ }
+
+ @Override
+ protected Long getJdoResult(GetHelper<Long> 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 3e50db451f9..ecac7d557ba 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 @@ public interface RawStore extends Configurable {
void addPrimaryKeys(List<SQLPrimaryKey> pks) throws InvalidObjectException,
MetaException;
void addForeignKeys(List<SQLForeignKey> 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 6e286d58c80..15e4e6b13ab 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 class HBaseStore implements RawStore {
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 ec3b2376dd7..b66019da24f 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 class DummyRawStoreControlledCommit implements
RawStore, Configurable {
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 a0f071a042b..41b4456e9ce 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 class DummyRawStoreForJdoConnection implements
RawStore {
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 978df5e4142..fbfb23ffcf3 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.client.builder.TableBuilder;
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 class TestTablesCreateDropAlterTruncate {
@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 class TestTablesCreateDropAlterTruncate {
@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 class TestTablesCreateDropAlterTruncate {
*/
@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 9b4fd704e96..de3e565da28 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 abstract class AbstractMetaStoreService {
*/
public void stop() {
}
+
+ public Configuration getConf() {
+ return configuration;
+ }
}
