manuzhang commented on code in PR #15240:
URL: https://github.com/apache/iceberg/pull/15240#discussion_r2871278732
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkCatalog.java:
##########
@@ -939,94 +862,84 @@ private Pair<String, List<String>>
parseLocationString(String location) {
}
@SuppressWarnings("CyclomaticComplexity")
- private Table loadFromPathIdentifier(PathIdentifier ident) {
+ private Table loadPath(PathIdentifier ident, TimeTravel timeTravel) throws
NoSuchTableException {
Pair<String, List<String>> parsed = parseLocationString(ident.location());
String metadataTableName = null;
- Long asOfTimestamp = null;
- Long snapshotId = null;
String branch = null;
- String tag = null;
+ TimeTravel effectiveTimeTravel = timeTravel;
boolean isChangelog = false;
- boolean isRewrite = false;
for (String meta : parsed.second()) {
if (meta.equalsIgnoreCase(SparkChangelogTable.TABLE_NAME)) {
isChangelog = true;
continue;
}
- if (REWRITE.equals(meta)) {
- isRewrite = true;
- continue;
- }
-
if (MetadataTableType.from(meta) != null) {
metadataTableName = meta;
continue;
}
- Matcher at = AT_TIMESTAMP.matcher(meta);
- if (at.matches()) {
- asOfTimestamp = Long.parseLong(at.group(1));
- continue;
- }
-
- Matcher id = SNAPSHOT_ID.matcher(meta);
- if (id.matches()) {
- snapshotId = Long.parseLong(id.group(1));
- continue;
- }
-
- Matcher branchRef = BRANCH.matcher(meta);
- if (branchRef.matches()) {
- branch = branchRef.group(1);
+ Matcher branchSelector = BRANCH.matcher(meta);
+ if (branchSelector.matches()) {
+ branch = branchSelector.group(1);
continue;
}
- Matcher tagRef = TAG.matcher(meta);
- if (tagRef.matches()) {
- tag = tagRef.group(1);
+ TimeTravel timeTravelSelector = parseTimeTravelSelector(meta);
+ if (timeTravelSelector != null) {
+ Preconditions.checkArgument(
+ timeTravel == null,
+ "Can't time travel using selector and Spark time travel spec at
the same time");
+ Preconditions.checkArgument(
+ effectiveTimeTravel == null,
+ "Can't time travel using multiple time travel selectors: (%s, %s)",
+ effectiveTimeTravel,
+ timeTravelSelector);
+ effectiveTimeTravel = timeTravelSelector;
}
}
- Preconditions.checkArgument(
- Stream.of(snapshotId, asOfTimestamp, branch,
tag).filter(Objects::nonNull).count() <= 1,
- "Can specify only one of snapshot-id (%s), as-of-timestamp (%s),
branch (%s), tag (%s)",
- snapshotId,
- asOfTimestamp,
- branch,
- tag);
-
- Preconditions.checkArgument(
- !isChangelog || (snapshotId == null && asOfTimestamp == null),
- "Cannot specify snapshot-id and as-of-timestamp for changelogs");
-
- org.apache.iceberg.Table table =
- tables.load(parsed.first() + (metadataTableName != null ? "#" +
metadataTableName : ""));
+ org.apache.iceberg.Table table;
+ try {
+ String qualifier = metadataTableName != null ? "#" + metadataTableName :
"";
Review Comment:
Can we add a constant like `METADATA_TABLE_SEPARATOR_IN_PATH`?
--
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]
