u70b3 opened a new pull request, #9037:
URL: https://github.com/apache/paimon/pull/9037

   ## What
   
   Fixes #9035 — four defects in the two watermark binary searches in 
`SnapshotManager` (`laterOrEqualWatermark` and `earlierOrEqualWatermark`, the 
latter a verbatim copy of the same code).
   
   These were surfaced while implementing `scan.watermark` batch time travel 
for paimon-rust (apache/paimon-rust#677) and cross-validating the behavior 
against the Java reference implementation.
   
   ## Defects fixed (both methods unless noted)
   
   1. **NPE in the guard** — `snapshot(latest).watermark() == Long.MIN_VALUE` 
unboxes a null `Long` when the latest snapshot has no watermark. This hits any 
table whose snapshots never carried a watermark (pure batch/Spark-written, or 
written by paimon-rust / pypaimon) queried with `scan.watermark`. The guard is 
now null-safe; the `MIN_VALUE` short-circuit is preserved.
   2. **Infinite loop** — the null-watermark fallback traversal decremented 
`mid` itself and could read *left of the search window*, so the window update 
recomputed its previous value and the search never terminated (scan thread 
hangs, re-reading snapshot files on every iteration). Interleaved null 
watermarks arise naturally on mixed-engine tables (Flink streaming writes with 
watermarks interleaved with watermark-less appends from other engines). The 
fallback now walks a separate `pos` bounded at `pos > earliest`; `mid` is never 
mutated, so the window provably shrinks every iteration.
   3. **Wrong snapshot recorded** — after the fallback, `finalSnapshot = 
snapshot` recorded the *original* mid snapshot (whose own watermark may be 
null) instead of the snapshot the fallback landed on, violating the method 
contract. Now records the landed-on snapshot; window updates use the original 
`mid` (`earliest = mid + 1`) and the fallback position (`latest = pos - 1`).
   4. **`earlierOrEqualWatermark` only: inverted early-return** — `if 
(earliestWatermark >= watermark) return snapshot(earliest);` was copied from 
`laterOrEqualWatermark` and is semantically inverted for earlier-or-equal. With 
the request below every snapshot's watermark it returned a snapshot violating 
the predicate instead of `null`, and the `rollback_to_watermark` procedures 
would then silently roll the table back to a *newer* state than requested 
(under-rollback). Now returns `null`, mirroring `earlierOrEqualTimeMills`, so 
the procedure fails loudly with "count not find snapshot".
   
   ## Behavior changes to be aware of
   
   - Below-minimum requests to `earlierOrEqualWatermark` now return `null` 
instead of the earliest snapshot (defect 4, intended).
   - Side effect of the same fix: with duplicate watermarks at the range start 
and the request equal to the earliest watermark, the result shifts from the 
first duplicate to a later duplicate via the binary search's exact-match 
branch. Both snapshots carry the requested watermark, so the contract is still 
satisfied, and this matches how the exact-match branch already behaves 
everywhere else in all four search methods (`laterOrEqualTimeMills` / 
`earlierOrEqualTimeMills` included).
   
   ## Testing
   
   New regression tests in `SnapshotManagerTest` — each of them fails (or 
hangs) on the old code:
   
   - interleaved null watermarks: termination + correct results for both 
methods (`@Timeout`-guarded so defective code fails fast instead of hanging CI)
   - exact match among null watermarks returns the watermark-bearing snapshot, 
not a null-watermark neighbor
   - all-null watermark tables return `null` (previously NPE at the guard)
   - null-watermark tail does not extend the searchable watermark range
   - below-minimum request returns `null` for `earlierOrEqualWatermark`
   
   The pre-existing `testEarlierOrEqualWatermark` race-mode expectation encoded 
defect 4 (it expected a snapshot whose watermark exceeds the request); updated 
to expect `null`, consistent with the fixed contract.
   
   - `mvn -pl paimon-core -am test -Dtest=SnapshotManagerTest`: 27 tests, 0 
failures, 0 errors
   - `mvn -pl paimon-core spotless:check`: passes
   


-- 
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]

Reply via email to