yihua commented on code in PR #7445:
URL: https://github.com/apache/hudi/pull/7445#discussion_r1055854052
##########
hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/HoodieMetaSyncOperations.java:
##########
@@ -89,6 +89,15 @@ default void dropPartitions(String tableName, List<String>
partitionsToDrop) {
}
+ /**
+ * Get location for the table in the metastore.
+ * @param tableName
+ * @return
+ */
+ default Option<String> getMetastoreLocation(String tableName) {
+ return Option.empty();
+ }
+
Review Comment:
Same here. This is only used in the tests. Could you avoid adding the new
API?
##########
hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/util/SparkDataSourceTableUtils.java:
##########
@@ -98,8 +98,15 @@ public static Map<String, String>
getSparkTableProperties(List<String> partition
public static Map<String, String> getSparkSerdeProperties(boolean
readAsOptimized, String basePath) {
Map<String, String> sparkSerdeProperties = new HashMap<>();
- sparkSerdeProperties.put("path", basePath);
+ Map<String, String> sparkSerdePathProperties =
getSparkSerdePathProperties(basePath);
Review Comment:
This change introduces the instantiation of a new Map and is not necessary.
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HoodieHiveSyncClient.java:
##########
@@ -248,6 +250,20 @@ public Option<String> getLastCommitTimeSynced(String
tableName) {
}
}
+ @Override
+ public Option<String> getMetastoreLocation(String tableName) {
+ try {
+ Table table = client.getTable(databaseName, tableName);
+ StorageDescriptor sd = table.getSd();
+ return Option.ofNullable(sd.getLocation());
+ } catch (NoSuchObjectException e) {
+ LOG.warn("the said table not found in hms " + tableId(databaseName,
tableName));
+ return Option.empty();
+ } catch (Exception e) {
+ throw new HoodieHiveSyncException("Failed to get the metastore location
from the table " + tableName, e);
+ }
+ }
Review Comment:
This is only used in the tests. Could you avoid adding the new API? Instead,
you should use `client = Hive.get(config.getHiveConf()).getMSC()` to get the
client in the test and then call this logic to get the table location and
verify it. The logic here should be moved to the test class.
--
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]
