stevenzwu commented on code in PR #13555:
URL: https://github.com/apache/iceberg/pull/13555#discussion_r2220684233
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/SparkCachedTableCatalog.java:
##########
@@ -128,74 +128,120 @@ 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());
- 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);
+ }
+ }
+
+ /** Extracts table load options from metadata. */
+ private TableLoadOptions parseLoadOptions(List<String> metadata) {
+ TableLoadOptions opts = new TableLoadOptions();
for (String meta : metadata) {
Review Comment:
this loop will does the pattern match for every element in the list. I
thought we only want to check the last 'selector` string. e.g. if the database
or table name contains any of the prefix (at_timestamp_ etc.), could this
result in false match?
--
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
