amogh-jahagirdar commented on code in PR #13555:
URL: https://github.com/apache/iceberg/pull/13555#discussion_r2219491069
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkCachedTableCatalog.java:
##########
@@ -128,74 +128,124 @@ public String name() {
return name;
}
- private Pair<Table, Long> load(Identifier ident) throws NoSuchTableException
{
+ private SparkTable load(Identifier ident) throws NoSuchTableException {
Preconditions.checkArgument(
ident.namespace().length == 0, CLASS_NAME + " does not support
namespaces");
Pair<String, List<String>> parsedIdent = parseIdent(ident);
String key = parsedIdent.first();
- List<String> metadata = parsedIdent.second();
+ TableLoadOptions options = parseLoadOptions(parsedIdent.second());
+ validateTableOptions(options);
- Long asOfTimestamp = null;
- Long snapshotId = null;
- String branch = null;
- String tag = null;
+ Table table = TABLE_CACHE.get(key);
+
+ if (table == null) {
+ throw new NoSuchTableException(ident);
+ }
+
+ if (options.isTableRewrite()) {
+ return new SparkTable(table, null, false, true);
+ }
+
+ if (options.snapshotId() != null) {
+ return new SparkTable(table, options.snapshotId(), false);
+ } else if (options.asOfTimestamp() != null) {
+ return new SparkTable(
+ table, SnapshotUtil.snapshotIdAsOfTime(table,
options.asOfTimestamp()), false);
+ } else if (options.branch() != null) {
+ Snapshot branchSnapshot = table.snapshot(options.branch());
+ Preconditions.checkArgument(
+ branchSnapshot != null,
+ "Cannot find snapshot associated with branch name: %s",
+ options.branch());
+ return new SparkTable(table, branchSnapshot.snapshotId(), false);
+ } else if (options.tag() != null) {
+ Snapshot tagSnapshot = table.snapshot(options.tag());
+ Preconditions.checkArgument(
+ tagSnapshot != null, "Cannot find snapshot associated with tag name:
%s", options.tag());
+ return new SparkTable(table, tagSnapshot.snapshotId(), false);
+ } else {
+ return new SparkTable(table, false);
+ }
+ }
+
+ private static class TableLoadOptions {
+ private Long asOfTimestamp;
+ private Long snapshotId;
+ private String branch;
+ private String tag;
+ private Boolean isTableRewrite;
+
+ Long asOfTimestamp() {
+ return asOfTimestamp;
+ }
+
+ Long snapshotId() {
+ return snapshotId;
+ }
+
+ String branch() {
+ return branch;
+ }
+
+ String tag() {
+ return tag;
+ }
+
+ boolean isTableRewrite() {
+ return Boolean.TRUE.equals(isTableRewrite);
+ }
+ }
Review Comment:
To reduce the cyclomatic complexity, I refactored this too so that it
delegates to a helper to parse the options into a structure and after
validating that, will return the appropriate `SparkTable`.
--
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]
