amogh-jahagirdar commented on code in PR #9298:
URL: https://github.com/apache/iceberg/pull/9298#discussion_r1442241575
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/BaseFileRewriteCoordinator.java:
##########
@@ -72,18 +70,12 @@ public void clearRewrite(Table table, String fileSetId) {
public Set<String> fetchSetIds(Table table) {
return resultMap.keySet().stream()
- .filter(e -> e.first().equals(tableUUID(table)))
+ .filter(e -> e.first().equals(Spark3Util.tableUUID(table)))
Review Comment:
Nit: I think it would be a bit cleaner just to import Spark3Util.tableUUID
and then just use tableUUID here (the diff on line 73 and other places would
essentially go away in favor of just a new import statement)
##########
spark/v3.4/spark/src/main/java/org/apache/iceberg/spark/Spark3Util.java:
##########
@@ -948,6 +950,17 @@ public static
org.apache.spark.sql.catalyst.TableIdentifier toV1TableIdentifier(
return org.apache.spark.sql.catalyst.TableIdentifier.apply(table,
database);
}
+ static String tableUUID(org.apache.iceberg.Table table) {
+ if (table instanceof HasTableOperations) {
+ TableOperations ops = ((HasTableOperations) table).operations();
+ return ops.current().uuid();
+ } else if (table instanceof BaseMetadataTable) {
+ return ((BaseMetadataTable) table).table().operations().current().uuid();
+ } else {
+ return null;
Review Comment:
I think this should probably throw an exception instead of returning null.
--
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
