amogh-jahagirdar commented on code in PR #13555:
URL: https://github.com/apache/iceberg/pull/13555#discussion_r2220871175
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -129,6 +129,7 @@ public class SparkCatalog extends BaseCatalog {
private static final Pattern SNAPSHOT_ID =
Pattern.compile("snapshot_id_(\\d+)");
private static final Pattern BRANCH = Pattern.compile("branch_(.*)");
private static final Pattern TAG = Pattern.compile("tag_(.*)");
+ private static final Pattern REWRITE_JOB = Pattern.compile("rewrite_(.*)");
Review Comment:
Ah all the logic in this class shouldn't even need to be defined for
`SparkCatalog`, I think I can safely remove all this.
We'll never access this code path in the current implementation of
compaction; compaction has an inherent expectation that the task group being
compacted has a reference in the `SparkTableCache`, and as a result compaction
will always internally use the `SparkCachingCatalog`.
https://github.com/apache/iceberg/blob/main/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/SparkDataFileRewriteRunner.java#L49
The identifier matching being done here should only be for identifiers that
are publicly facing users but for compaction it'll look something like
`<compaction groupId>#rewrite` where rewrite is a part of the identifier
selector metadata,. where the group ID is something we assign internally and
isn't expected that a user should directly try to reference
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -129,6 +129,7 @@ public class SparkCatalog extends BaseCatalog {
private static final Pattern SNAPSHOT_ID =
Pattern.compile("snapshot_id_(\\d+)");
private static final Pattern BRANCH = Pattern.compile("branch_(.*)");
private static final Pattern TAG = Pattern.compile("tag_(.*)");
+ private static final Pattern REWRITE_JOB = Pattern.compile("rewrite_(.*)");
Review Comment:
Ah all the logic in this class shouldn't even need to be defined for
`SparkCatalog`, I think I can safely remove all this.
We'll never access this code path in the current implementation of
compaction; compaction has an inherent expectation that the task group being
compacted has a reference in the `SparkTableCache`, and as a result compaction
will always internally use the `SparkCachingCatalog`.
https://github.com/apache/iceberg/blob/main/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/SparkDataFileRewriteRunner.java#L49
The identifier matching being done here should only be for identifiers that
are publicly facing users but for compaction it'll look something like
`<compaction groupId>#rewrite` where rewrite is a part of the identifier
selector metadata,. and the group ID is something we assign internally and
isn't expected that a user should directly try to reference
--
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]
