HIVE-26882: Allow transactional check of Table parameter before altering the Table (#3888) (Peter Vary reviewed by Prasanth Jayachandran and Szehon Ho)

metastore/if/hive_metastore.thrift

@@ -1550,3 +1550,6 @@ const string TABLE_NO_AUTO_COMPACT = "no_auto_compaction",
 const string TABLE_TRANSACTIONAL_PROPERTIES = "transactional_properties",
 
 
+// Keys for alter table environment context parameters
+const string EXPECTED_PARAMETER_KEY = "expected_parameter_key",
+const string EXPECTED_PARAMETER_VALUE = "expected_parameter_value",

metastore/src/java/org/apache/hadoop/hive/metastore/DefaultPartitionExpressionProxy.java

@@ -0,0 +1,56 @@
+package org.apache.hadoop.hive.metastore;
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.
+ */
+
+import org.apache.hadoop.hive.metastore.api.FileMetadataExprType;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.ql.io.sarg.SearchArgument;
+import org.apache.hadoop.hive.serde2.typeinfo.PrimitiveTypeInfo;
+
+import java.util.List;
+
+/**
+ * Default implementation of PartitionExpressionProxy.  Eventually this should use the SARGs in
+ * Hive's storage-api.  For now it just throws UnsupportedOperationException.
+ */
+public class DefaultPartitionExpressionProxy implements PartitionExpressionProxy {
+    @Override
+    public String convertExprToFilter(byte[] expr) throws MetaException {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public boolean filterPartitionsByExpr(List<String> partColumnNames, List<PrimitiveTypeInfo> partColumnTypeInfos, byte[] expr, String defaultPartitionName, List<String> partitionNames) throws MetaException {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public FileMetadataExprType getMetadataType(String inputFormat) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public FileFormatProxy getFileFormatProxy(FileMetadataExprType type) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public SearchArgument createSarg(byte[] expr) {
+        throw new UnsupportedOperationException();
+    }
+}

metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java

@@ -51,6 +51,7 @@
 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;
@@ -149,13 +150,29 @@ public void alterTable(RawStore msdb, Warehouse wh, String dbname,
         rename = true;
       }
 
-      msdb.openTransaction();
+      String expectedKey = environmentContext != null && environmentContext.getProperties() != null ?
+              environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY) : null;
+      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();
+      }
       // get old table
       oldt = msdb.getTable(dbname, name);
       if (oldt == null) {
         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 (oldt.getPartitionKeysSize() != 0) {
         isPartitionedTable = true;
       }

metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java

@@ -561,17 +561,31 @@ public void shutdown() {
   }
 
   /**
-   * Opens a new one or the one already created Every call of this function must
-   * have corresponding commit or rollback function call
+   * Opens a new one or the one already created. Every call of this function must
+   * have corresponding commit or rollback function call.
    *
    * @return an active transaction
    */
-
   @Override
   public boolean openTransaction() {
+    return openTransaction(null);
+  }
+
+  /**
+   * Opens a new one or the one already created. Every call of this function must
+   * have corresponding commit or rollback function call.
+   *
+   * @param isolationLevel The transaction isolation level. Only possible to set on the first call.
+   * @return an active transaction
+   */
+  @Override
+  public boolean openTransaction(String isolationLevel) {
     openTrasactionCalls++;
     if (openTrasactionCalls == 1) {
       currentTransaction = pm.currentTransaction();
+      if (isolationLevel != null) {
+        currentTransaction.setIsolationLevel(isolationLevel);
+      }
       currentTransaction.begin();
       transactionStatus = TXN_STATUS.OPEN;
     } else {
@@ -581,10 +595,16 @@ public boolean openTransaction() {
         throw new RuntimeException("openTransaction called in an interior"
             + " transaction scope, but currentTransaction is not active.");
       }
+
+      // Can not change the isolation level on an already open transaction
+      if (isolationLevel != null && !isolationLevel.equals(currentTransaction.getIsolationLevel())) {
+        throw new RuntimeException("Can not set isolation level on an open transaction");
+      }
     }
 
     boolean result = currentTransaction.isActive();
-    debugLog("Open transaction: count = " + openTrasactionCalls + ", isActive = " + result);
+    debugLog("Open transaction: count = " + openTrasactionCalls + ", isActive = " + result + ", isolationLevel = "
+            + currentTransaction.getIsolationLevel());
     return result;
   }
 

metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java

@@ -77,13 +77,23 @@ public interface RawStore extends Configurable {
   public abstract void shutdown();
 
   /**
-   * Opens a new one or the one already created Every call of this function must
-   * have corresponding commit or rollback function call
+   * Opens a new one or the one already created. Every call of this function must
+   * have corresponding commit or rollback function call.
    *
    * @return an active transaction
    */
+  boolean openTransaction();
 
-  public abstract boolean openTransaction();
+  /**
+   * Opens a new one or the one already created. Every call of this function must
+   * have corresponding commit or rollback function call.
+   *
+   * @param isolationLevel The transaction isolation level. Only possible to set on the first call.
+   * @return an active transaction
+   */
+  default boolean openTransaction(String isolationLevel) {
+    throw new UnsupportedOperationException("Setting isolation level for this Store is not supported");
+  }
 
   /**
    * if this is the commit of the first open call then an actual commit is

metastore/src/java/org/apache/hadoop/hive/metastore/client/builder/ConstraintBuilder.java

@@ -0,0 +1,98 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.client.builder;
+
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.api.Table;
+
+/**
+ * Base builder for all types of constraints.  Database name, table name, and column name
+ * must be provided.
+ * @param <T> Type of builder extending this.
+ */
+abstract class ConstraintBuilder<T> {
+  protected String dbName, tableName, columnName, constraintName;
+  protected int keySeq;
+  protected boolean enable, validate, rely;
+  private T child;
+
+  protected ConstraintBuilder() {
+    keySeq = 1;
+    enable = true;
+    validate = rely = false;
+  }
+
+  protected void setChild(T child) {
+    this.child = child;
+  }
+
+  protected void checkBuildable(String defaultConstraintName) throws MetaException {
+    if (dbName == null || tableName == null || columnName == null) {
+      throw new MetaException("You must provide database name, table name, and column name");
+    }
+    if (constraintName == null) {
+      constraintName = dbName + "_" + tableName + "_" + columnName + "_" + defaultConstraintName;
+    }
+  }
+
+  public T setDbName(String dbName) {
+    this.dbName = dbName;
+    return child;
+  }
+
+  public T setTableName(String tableName) {
+    this.tableName = tableName;
+    return child;
+  }
+
+  public T setDbAndTableName(Table table) {
+    this.dbName = table.getDbName();
+    this.tableName = table.getTableName();
+    return child;
+  }
+
+  public T setColumnName(String columnName) {
+    this.columnName = columnName;
+    return child;
+  }
+
+  public T setConstraintName(String constraintName) {
+    this.constraintName = constraintName;
+    return child;
+  }
+
+  public T setKeySeq(int keySeq) {
+    this.keySeq = keySeq;
+    return child;
+  }
+
+  public T setEnable(boolean enable) {
+    this.enable = enable;
+    return child;
+  }
+
+  public T setValidate(boolean validate) {
+    this.validate = validate;
+    return child;
+  }
+
+  public T setRely(boolean rely) {
+    this.rely = rely;
+    return child;
+  }
+}