This is an automated email from the ASF dual-hosted git repository.
dkuzmenko pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/branch-4.0 by this push:
new 24018002923 HIVE-28121: Use direct SQL for transactional altering
table parameter (Rui Li, reviewed by Denys Kuzmenko)
24018002923 is described below
commit 24018002923912f320a4cfde6c4b475c71e29d90
Author: Rui Li <[email protected]>
AuthorDate: Sat Apr 20 04:14:42 2024 +0800
HIVE-28121: Use direct SQL for transactional altering table parameter (Rui
Li, reviewed by Denys Kuzmenko)
Closes #5197
---
.../hadoop/hive/metastore/HiveAlterHandler.java | 26 +++++++++++++---------
.../hadoop/hive/metastore/MetaStoreDirectSql.java | 10 +++++++++
.../apache/hadoop/hive/metastore/ObjectStore.java | 23 +++++++++++++++++++
.../org/apache/hadoop/hive/metastore/RawStore.java | 10 +++++++++
.../client/TestTablesCreateDropAlterTruncate.java | 7 ++++++
5 files changed, 65 insertions(+), 11 deletions(-)
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
index a2807961b75..53af0a5f302 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
@@ -56,7 +56,6 @@ import org.apache.hadoop.hive.metastore.api.Partition;
import org.apache.hadoop.hive.metastore.api.Table;
import org.apache.hadoop.hive.metastore.api.hive_metastoreConstants;
-import javax.jdo.Constants;
import java.io.IOException;
import java.net.URI;
import java.util.ArrayList;
@@ -183,12 +182,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
// Note: we don't verify stats here; it's done below in
alterTableUpdateTableColumnStats.
olddb = msdb.getDatabase(catName, dbname);
@@ -198,10 +192,20 @@ public class HiveAlterHandler implements AlterHandler {
TableName.getQualified(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");
+ }
}
validateTableChangesOnReplSource(olddb, oldt, newt, environmentContext);
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
index c453df0ea1b..204aafc89d5 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
@@ -53,6 +53,7 @@ import javax.jdo.Query;
import javax.jdo.Transaction;
import javax.jdo.datastore.JDOConnection;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.conf.Configuration;
@@ -3273,4 +3274,13 @@ class MetaStoreDirectSql {
return result;
}
}
+
+ long updateTableParam(Table table, String key, String expectedValue, String
newValue) {
+ String statement = TxnUtils.createUpdatePreparedStmt(
+ "\"TABLE_PARAMS\"",
+ ImmutableList.of("\"PARAM_VALUE\""),
+ ImmutableList.of("\"TBL_ID\"", "\"PARAM_KEY\"", "\"PARAM_VALUE\""));
+ Query query = pm.newQuery("javax.jdo.query.SQL", statement);
+ return (long) query.executeWithArray(newValue, table.getId(), key,
expectedValue);
+ }
}
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
index a810c9cc695..ce4e96927b7 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
@@ -624,6 +624,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, InvalidObjectException {
+ 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/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
index 5dead2d425d..5ab2907f5ba 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
@@ -2317,4 +2317,14 @@ public interface RawStore extends Configurable {
return null;
}
+ /**
+ * 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/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
index cf4fc482808..6e0616713f8 100644
---
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
+++
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
@@ -63,6 +63,7 @@ import org.apache.thrift.TException;
import org.apache.thrift.protocol.TProtocolException;
import org.junit.After;
import org.junit.Assert;
+import org.junit.Assume;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -1196,6 +1197,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();
@@ -1209,6 +1212,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();
@@ -1228,6 +1233,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");
