nk1506 commented on code in PR #9852:
URL: https://github.com/apache/iceberg/pull/9852#discussion_r1520888231
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveOperationsBase.java:
##########
@@ -181,4 +296,214 @@ default Table newHmsTable(String hmsTableOwner) {
return newTable;
}
+
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
+ default void commitWithLocking(
+ Configuration conf,
+ BaseMetadata base,
+ BaseMetadata metadata,
+ String baseMetadataLocation,
+ String newMetadataLocation,
+ FileIO io) {
+ boolean newTable = base == null;
+ boolean hiveEngineEnabled = hiveEngineEnabled(metadata.properties());
+
+ BaseMetastoreOperations.CommitStatus commitStatus =
+ BaseMetastoreOperations.CommitStatus.FAILURE;
+ boolean updateHiveTable = false;
+ HiveLock lock = lockObject(metadata.properties(), conf, catalogName());
+ try {
+ lock.lock();
+ Table tbl = loadHmsTable();
+
+ if (tbl != null) {
+ String tableType = tbl.getTableType();
+ if (!tableType.equalsIgnoreCase(tableType().name())) {
+ throw new AlreadyExistsException(
+ "%s with same name already exists: %s.%s",
+ tableType.equalsIgnoreCase(TableType.VIRTUAL_VIEW.name()) ?
"View" : "Table",
+ tbl.getDbName(),
+ tbl.getTableName());
+ }
+
+ // If we try to create the table but the metadata location is already
set, then we had a
+ // concurrent commit
+ if (newTable
+ &&
tbl.getParameters().get(BaseMetastoreTableOperations.METADATA_LOCATION_PROP)
+ != null) {
+ throw new AlreadyExistsException(
+ "%s already exists: %s.%s", contentType().value(), database(),
table());
+ }
+
+ updateHiveTable = true;
+ LOG.debug("Committing existing {}: {}",
contentType().value().toLowerCase(), fullName());
+ } else {
+ tbl =
+ newHmsTable(
+ metadata
+ .properties()
+ .getOrDefault(HiveCatalog.HMS_TABLE_OWNER,
HiveHadoopUtil.currentUser()));
+ LOG.debug("Committing new {}: {}",
contentType().value().toLowerCase(), fullName());
+ }
+
+ tbl.setSd(
+ storageDescriptor(
+ metadata.schema(),
+ metadata.location(),
+ hiveEngineEnabled)); // set to pickup any schema changes
+
+ String metadataLocation =
+
tbl.getParameters().get(BaseMetastoreTableOperations.METADATA_LOCATION_PROP);
+
+ if (!Objects.equals(baseMetadataLocation, metadataLocation)) {
+ throw new CommitFailedException(
+ "Cannot commit: Base metadata location '%s' is not same as the
current %s metadata location '%s' for %s.%s",
+ baseMetadataLocation,
+ contentType().value().toLowerCase(),
+ metadataLocation,
+ database(),
+ table());
+ }
+
+ Map<String, String> baseProperties = base == null ? null :
base.properties();
+ setHmsParameters(
Review Comment:
```suggestion
switch (contentType()) {
case TABLE:
setHmsParameters(
metadata,
tbl,
newMetadataLocation,
obsoleteProps(conf, baseProperties, metadata.properties()),
hiveEngineEnabled);
case VIEW:
setCommonHmsParameters(tbl,
HiveOperationsBase.ICEBERG_VIEW_TYPE_VALUE.toUpperCase(Locale.ENGLISH),
newMetadataLocation,
metadata.schema(),
((ViewMetadata) metadata).uuid(),
obsoleteProps(conf, baseProperties, metadata.properties()),
metadata::metadataFileLocation);
}
```
Since Hive-Table is very dependent on Metadata object
```
setSnapshotStats(metadata, parameters);
setPartitionSpec(metadata, parameters);
setSortOrder(metadata, parameters);
```
Should we add the conditional statement? Else we will have to make many
changes for related parts.
@szehon-ho
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
