[ https://issues.apache.org/jira/browse/HIVE-26882?focusedWorklogId=837001&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-837001 ]
ASF GitHub Bot logged work on HIVE-26882: ----------------------------------------- Author: ASF GitHub Bot Created on: 04/Jan/23 18:45 Start Date: 04/Jan/23 18:45 Worklog Time Spent: 10m Work Description: prasanthj commented on code in PR #3888: URL: https://github.com/apache/hive/pull/3888#discussion_r1061784678 ########## standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java: ########## @@ -1183,6 +1193,90 @@ public void testAlterTableAlreadyExists() throws Exception { } } + @Test + public void testAlterTableExpectedPropertyMatch() throws Exception { + Table originalTable = testTables[0]; + + EnvironmentContext context = new EnvironmentContext(); + context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_KEY, "transient_lastDdlTime"); + context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE, + originalTable.getParameters().get("transient_lastDdlTime")); + + client.alter_table(originalTable.getCatName(), originalTable.getDbName(), originalTable.getTableName(), + originalTable, context); + } + + @Test(expected = MetaException.class) + public void testAlterTableExpectedPropertyDifferent() throws Exception { + Table originalTable = testTables[0]; + + EnvironmentContext context = new EnvironmentContext(); + context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_KEY, "transient_lastDdlTime"); + context.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE, "alma"); + + client.alter_table(originalTable.getCatName(), originalTable.getDbName(), originalTable.getTableName(), + originalTable, context); + } + + /** + * This tests ensures that concurrent Iceberg commits will fail. Acceptable as a first sanity check. + * <p> + * I have not found a good way to check that HMS side database commits are parallel in the + * {@link org.apache.hadoop.hive.metastore.HiveAlterHandler#alterTable(RawStore, Warehouse, String, String, String, Table, EnvironmentContext, IHMSHandler, String)} + * call, but this test could be used to manually ensure that using breakpoints. + */ + @Test + public void testAlterTableExpectedPropertyConcurrent() throws Exception { + Table originalTable = testTables[0]; + + originalTable.getParameters().put("snapshot", "1"); + client.alter_table(originalTable.getCatName(), originalTable.getDbName(), originalTable.getTableName(), + originalTable, null); + + EnvironmentContext context = new EnvironmentContext(); Review Comment: nit: small change to concurrency test. if you can run a loop for few iterations with snapshot=i and expected value=i-1 that will be useful.. just some repetition to avoid flakiness.. Issue Time Tracking ------------------- Worklog Id: (was: 837001) Time Spent: 1h 40m (was: 1.5h) > Allow transactional check of Table parameter before altering the Table > ---------------------------------------------------------------------- > > Key: HIVE-26882 > URL: https://issues.apache.org/jira/browse/HIVE-26882 > Project: Hive > Issue Type: Improvement > Components: Standalone Metastore > Reporter: Peter Vary > Priority: Major > Labels: pull-request-available > Time Spent: 1h 40m > Remaining Estimate: 0h > > We should add the possibility to transactionally check if a Table parameter > is changed before altering the table in the HMS. > This would provide an alternative, less error-prone and faster way to commit > an Iceberg table, as the Iceberg table currently needs to: > - Create an exclusive lock > - Get the table metadata to check if the current snapshot is not changed > - Update the table metadata > - Release the lock > After the change these 4 HMS calls could be substituted with a single alter > table call. > Also we could avoid cases where the locks are left hanging by failed processes -- This message was sent by Atlassian Jira (v8.20.10#820010)