This is an automated email from the ASF dual-hosted git repository.
lirui pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/branch-3.1 by this push:
new bdf3285bc25 HIVE-28121: Use direct SQL for transactional altering
table parameter (Rui Li, reviewed by Peter Vary) (#5208)
bdf3285bc25 is described below
commit bdf3285bc250609c1b31a52e7732be8c043b8817
Author: Rui Li <[email protected]>
AuthorDate: Tue May 21 12:26:00 2024 +0800
HIVE-28121: Use direct SQL for transactional altering table parameter (Rui
Li, reviewed by Peter Vary) (#5208)
---
.../hadoop/hive/metastore/HiveAlterHandler.java | 25 +++++++++++++---------
.../hadoop/hive/metastore/MetaStoreDirectSql.java | 9 ++++++++
.../apache/hadoop/hive/metastore/ObjectStore.java | 23 ++++++++++++++++++++
.../org/apache/hadoop/hive/metastore/RawStore.java | 9 ++++++++
.../hadoop/hive/metastore/conf/MetastoreConf.java | 5 +++++
.../client/TestTablesCreateDropAlterTruncate.java | 10 ++++++++-
6 files changed, 70 insertions(+), 11 deletions(-)
diff --git
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
index f010f0d2cca..cc96de8a636 100644
---
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
+++
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
@@ -152,12 +152,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();
// get old table
oldt = msdb.getTable(catName, dbname, name);
if (oldt == null) {
@@ -165,10 +160,20 @@ public class HiveAlterHandler implements AlterHandler {
Warehouse.getCatalogQualifiedTableName(catName, 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");
+ }
}
if (oldt.getPartitionKeysSize() != 0) {
diff --git
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
index 4df43d628c3..0762a64a7b8 100644
---
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
+++
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
@@ -2555,4 +2555,13 @@ class MetaStoreDirectSql {
query.closeAll();
}
}
+
+ 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 %s JOIN %s ON %s.\"DB_ID\" = %s.\"DB_ID\"
WHERE \"TBL_NAME\" = '%s' AND \"NAME\" = '%s')",
+ TBLS, DBS, TBLS, DBS, table.getTableName(), table.getDbName());
+ Query query = pm.newQuery("javax.jdo.query.SQL", queryText);
+ return (long) query.executeWithArray(newValue, key, expectedValue);
+ }
}
diff --git
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
index 815ee4184d8..3dbde7039dd 100644
---
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
+++
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
@@ -751,6 +751,29 @@ public class ObjectStore implements RawStore, Configurable
{
return result;
}
+ @Override
+ public long updateParameterWithExpectedValue(Table table, String key, String
expectedValue, String newValue)
+ throws MetaException, NoSuchObjectException {
+ return new GetHelper<Long>(table.getCatName(), 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/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
index a6ffcf417e9..8c41a40bcfa 100644
---
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
+++
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
@@ -1657,4 +1657,13 @@ public interface RawStore extends Configurable {
Map<String, List<String>> getPartitionColsWithStats(String catName, String
dbName,
String tableName) throws MetaException, NoSuchObjectException;
+ /**
+ * Updates a given table parameter with expected value.
+ *
+ * @return the number of rows updated
+ */
+ default 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/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
index b8aab6037cb..28f1c8f7720 100644
---
a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
+++
b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
@@ -419,6 +419,10 @@ public class MetastoreConf {
"Default transaction isolation level for identity generation."),
DATANUCLEUS_USE_LEGACY_VALUE_STRATEGY("datanucleus.rdbms.useLegacyNativeValueStrategy",
"datanucleus.rdbms.useLegacyNativeValueStrategy", true, ""),
+ DATANUCLEUS_QUERY_SQL_ALLOWALL("datanucleus.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."),
DBACCESS_SSL_PROPS("metastore.dbaccess.ssl.properties",
"hive.metastore.dbaccess.ssl.properties", "",
"Comma-separated SSL properties for metastore to access database when
JDO connection URL\n" +
"enables SSL access. e.g.
javax.net.ssl.trustStore=/tmp/truststore,javax.net.ssl.trustStorePassword=pwd."),
@@ -1108,6 +1112,7 @@ public class MetastoreConf {
ConfVars.DATANUCLEUS_PLUGIN_REGISTRY_BUNDLE_CHECK,
ConfVars.DATANUCLEUS_TRANSACTION_ISOLATION,
ConfVars.DATANUCLEUS_USE_LEGACY_VALUE_STRATEGY,
+ ConfVars.DATANUCLEUS_QUERY_SQL_ALLOWALL,
ConfVars.DETACH_ALL_ON_COMMIT,
ConfVars.IDENTIFIER_FACTORY,
ConfVars.MANAGER_FACTORY_CLASS,
diff --git
a/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
b/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
index a2ab9e807f3..f2d799b8baa 100644
---
a/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
+++
b/standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
@@ -49,6 +49,7 @@ import
org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder;
import org.apache.hadoop.hive.metastore.client.builder.PartitionBuilder;
import org.apache.hadoop.hive.metastore.client.builder.TableBuilder;
import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
import org.apache.hadoop.hive.metastore.minihms.AbstractMetaStoreService;
import org.apache.thrift.TApplicationException;
import org.apache.thrift.TException;
@@ -56,6 +57,7 @@ import org.apache.thrift.protocol.TProtocolException;
import org.apache.thrift.transport.TTransportException;
import org.junit.After;
import org.junit.Assert;
+import org.junit.Assume;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -105,7 +107,7 @@ public class TestTablesCreateDropAlterTruncate extends
MetaStoreClientTest {
@BeforeClass
public static void startMetaStores() {
- Map<MetastoreConf.ConfVars, String> msConf = new
HashMap<MetastoreConf.ConfVars, String>();
+ Map<ConfVars, String> msConf = new HashMap<ConfVars, String>();
// Enable trash, so it can be tested
Map<String, String> extraConf = new HashMap<>();
extraConf.put("fs.trash.checkpoint.interval", "30"); //
FS_TRASH_CHECKPOINT_INTERVAL_KEY
@@ -1086,6 +1088,8 @@ public class TestTablesCreateDropAlterTruncate extends
MetaStoreClientTest {
@Test
public void testAlterTableExpectedPropertyMatch() throws Exception {
+ Assume.assumeTrue(MetastoreConf.getBoolVar(metaStore.getConf(),
ConfVars.TRY_DIRECT_SQL));
+ Assume.assumeTrue(MetastoreConf.getBoolVar(metaStore.getConf(),
ConfVars.TRY_DIRECT_SQL_DDL));
Table originalTable = testTables[0];
EnvironmentContext context = new EnvironmentContext();
@@ -1099,6 +1103,8 @@ public class TestTablesCreateDropAlterTruncate extends
MetaStoreClientTest {
@Test(expected = MetaException.class)
public void testAlterTableExpectedPropertyDifferent() throws Exception {
+ Assume.assumeTrue(MetastoreConf.getBoolVar(metaStore.getConf(),
ConfVars.TRY_DIRECT_SQL));
+ Assume.assumeTrue(MetastoreConf.getBoolVar(metaStore.getConf(),
ConfVars.TRY_DIRECT_SQL_DDL));
Table originalTable = testTables[0];
EnvironmentContext context = new EnvironmentContext();
@@ -1118,6 +1124,8 @@ public class TestTablesCreateDropAlterTruncate extends
MetaStoreClientTest {
*/
@Test
public void testAlterTableExpectedPropertyConcurrent() throws Exception {
+ Assume.assumeTrue(MetastoreConf.getBoolVar(metaStore.getConf(),
ConfVars.TRY_DIRECT_SQL));
+ Assume.assumeTrue(MetastoreConf.getBoolVar(metaStore.getConf(),
ConfVars.TRY_DIRECT_SQL_DDL));
Table originalTable = testTables[0];
originalTable.getParameters().put("snapshot", "0");
