u70b3 commented on code in PR #9037:
URL: https://github.com/apache/paimon/pull/9037#discussion_r3725783969
##########
paimon-core/src/main/java/org/apache/paimon/utils/SnapshotManager.java:
##########
@@ -361,9 +361,13 @@ public boolean earliestFileNotExists() {
public @Nullable Snapshot earlierOrEqualWatermark(long watermark) {
Long latest = latestSnapshotId();
+ if (latest == null) {
+ return null;
+ }
// If latest == Long.MIN_VALUE don't need next binary search for
watermark
// which can reduce IO cost with snapshot
- if (latest == null || snapshot(latest).watermark() == Long.MIN_VALUE) {
+ Long latestWatermark = snapshot(latest).watermark();
+ if (latestWatermark != null && latestWatermark == Long.MIN_VALUE) {
Review Comment:
Thanks for the careful review! You're right — the sentinel was only filtered
on the latest snapshot, and with [Long.MIN_VALUE, null] the search would
incorrectly return the sentinel snapshot for a rollback.
Fixed in the latest commit: both null and Long.MIN_VALUE are now treated
uniformly as missing (new isMissingWatermark helper) in the initial scan and
the in-window fallback of both searches, and the latest-snapshot fast path is
removed since it also incorrectly short-circuited histories like [100,
Long.MIN_VALUE] where an earlier real watermark must still be found. Added
regression tests for [Long.MIN_VALUE, null] and for the sentinel mixed with
real watermarks.
--
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]
