jackylee-ch commented on PR #946: URL: https://github.com/apache/paimon-rust/pull/946#issuecomment-5943935140
Addressed by fixing the root cause rather than only bounding the C input. `LimitPushdownAccumulator::push` compared `scanned_row_count >= self.limit as i64`. Since `limit` is a `usize`, any value above `i64::MAX` (your `SIZE_MAX` case, or `INT64_MAX + 1`) turned negative and the first counted split satisfied it. It now compares as `u128`; `scanned_row_count` is always non-negative, so a limit a real row count cannot reach never early-stops. This makes the lossy cast harmless for every caller (C, Go, core), not just the one C entry point. Coverage: - Core: `test_incremental_limit_accumulator_does_not_truncate_on_oversized_limit` pushes two counted splits under `usize::MAX` and `i64::MAX + 1` and asserts neither early-stops and all splits are kept. - C boundary: `test_read_builder_with_limit_prunes_plan_splits` now also asserts `paimon_read_builder_with_limit` with `SIZE_MAX` and `i64::MAX + 1` retains every one of its multiple splits. I verified non-vacuity: restoring the `as i64` cast makes `SIZE_MAX` truncate to the first split and both tests fail; the `u128` comparison passes. Rebased onto current main. The accumulator tests and `paimon-c` (83 tests, incl. the FFI plan test) pass; `clippy -p paimon -p paimon-c --all-targets -D warnings` is clean. -- 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]
