zhongyujiang commented on code in PR #5590:
URL: https://github.com/apache/paimon/pull/5590#discussion_r2084645599
##########
paimon-core/src/main/java/org/apache/paimon/table/AbstractFileStoreTable.java:
##########
@@ -479,18 +479,13 @@ protected Runnable newExpireRunnable() {
}
private Optional<TableSchema> tryTimeTravel(Options options) {
- Snapshot snapshot;
- try {
- snapshot =
- TimeTravelUtil.tryTravelToSnapshot(options,
snapshotManager(), tagManager())
- .orElse(null);
- } catch (Exception e) {
- return Optional.empty();
- }
- if (snapshot == null) {
- return Optional.empty();
- }
- return
Optional.of(schemaManager().schema(snapshot.schemaId()).copy(options.toMap()));
+ Optional<Snapshot> snapshot =
+ TimeTravelUtil.tryTravelToSnapshot(options, snapshotManager(),
tagManager());
+
+ return snapshot.isPresent()
+ ? Optional.of(
+
schemaManager().schema(snapshot.get().schemaId()).copy(options.toMap()))
+ : Optional.empty();
Review Comment:
This was changed to throw an exception directly if the time travel fails to
find the snapshot.
But when I'm investigating the time travel releated test failures, I found
that it seems Paimon currently allows querying a non-existent snapshot and
returns an empty result.
Is this behavior reasonable? Shouldn't an exception be thrown if time travel
fails to find the snapshot?
https://github.com/apache/paimon/blob/408b78d0dfdd95263436e25a3312b482de714ecd/paimon-spark/paimon-spark-ut/src/test/scala/org/apache/paimon/spark/PaimonSourceTest.scala#L89-L123
--
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...@paimon.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
