szehon-ho commented on code in PR #3888:
URL: https://github.com/apache/hive/pull/3888#discussion_r1055913042


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -6038,17 +6039,26 @@ public void alter_table_with_environment_context(final 
String dbname,
       throws InvalidOperationException, MetaException {
     String[] parsedDbName = parseDbName(dbname, conf);
     alter_table_core(parsedDbName[CAT_NAME], parsedDbName[DB_NAME],
-        name, newTable, envContext, null, null, null);
+        name, newTable, envContext, null, null, null, null, null);
   }
 
   private void alter_table_core(String catName, String dbname, String name, 
Table newTable,
-                                EnvironmentContext envContext, String 
validWriteIdList, List<String> processorCapabilities, String processorId)
+                                EnvironmentContext envContext, String 
validWriteIdList, List<String> processorCapabilities,
+                                String processorId, String 
expectedPropertyKey, String expectedPropertyValue)

Review Comment:
   Did you consider allowing a list of table properties here?



##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java:
##########
@@ -187,6 +187,16 @@ public void alterTable(RawStore msdb, Warehouse wh, String 
catName, String dbnam
             TableName.getQualified(catName, dbname, name) + " doesn't exist");
       }
 
+      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 && expectedValue != null
+              && !expectedValue.equals(oldt.getParameters().get(expectedKey))) 
{
+        throw new MetaException("The table has been modified. The parameter 
value for key '" + expectedKey + "' is '"

Review Comment:
   For my understanding, this only works if a user has table lock while calling 
alter right?
   
   If user does not have lock, Hive has no internal lock to prevent two users 
from both getting their expected value and proceeding right?  If so, maybe 
worth to add a javadoc on this method.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to