JingsongLi commented on PR #946: URL: https://github.com/apache/paimon-rust/pull/946#issuecomment-5935945035
Thanks for fixing the negative Go limit and narrowing the planning claim. The Go regression and an additional Go → C → scan → reader probe now pass, including zero, small, large, maximum nonnegative `int`, rejected negative input and closed-builder handling. One P2 input-range issue remains in the newly exposed C API, `paimon_read_builder_with_limit` (`bindings/c/src/table.rs`, around lines 657–669). It accepts all `size_t` values and returns success, but the core `LimitPushdownAccumulator::push` converts the limit with `self.limit as i64`. On a 64-bit platform, `SIZE_MAX` or any value above `INT64_MAX` becomes negative, so the first known-count split satisfies the limit and subsequent splits are omitted. A very large positive upper bound must not silently truncate a small table. I extended the actual C FFI test after three real writes/commits (six rows across three splits). An ordinary large limit retains all three splits; `usize::MAX` is accepted but retains only one. The assertion fails on this head. A temporary control bounding the C-side value to `i64::MAX` passes. Please reject unsupported values before storing them, or make the accumulator compare/saturate without a lossy signed cast, with C-boundary coverage for `INT64_MAX + 1` and `SIZE_MAX`. Validation: all 83 existing C tests passed; the native library was rebuilt from this head and the two Go limit tests plus the additional real reader-chain probe passed without warehouse skips. `go vet` passed. The fixture was generated by native Rust writes; this was not a Spark compatibility run. -- 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]
