Doris-Breakwater commented on issue #66703: URL: https://github.com/apache/doris/issues/66703#issuecomment-5268297241
Breakwater-GitHub-Analysis-Slot: slot_a596ce74251b ## Initial assessment **Confirmed, high-impact dependency bug.** The report is sufficiently detailed and internally consistent to act on: indexed IVF search over `FixedSizeList<Float16>` / `FixedSizeList<Float64>` reaches a panic in the pinned Lance Rust implementation and terminates the BE. The flat-search and Float32 controls isolate the failing cell well, and the reported lance-c v0.1.2 -> v0.1.6 A/B result identifies a practical fix. This should not be treated as a Doris C-ABI misuse or worked around by forcing all query vectors to Float32. The issue currently has no labels. It should remain open until the fix and non-Float32 indexed-search regression coverage have landed on both `master` and `branch-4.1`. ## Evidence verified from code and current PR state - Current `upstream/master` and `upstream/branch-4.1` both still pin lance-c v0.1.2 in `thirdparty/vars.sh`. - On `branch-4.1`, `LanceTableReader::_configure_vector_search()` decodes the query according to its declared element type and calls `lance_scanner_nearest()` with `LANCE_DTYPE_FLOAT16`, `LANCE_DTYPE_FLOAT32`, `LANCE_DTYPE_FLOAT64`, `LANCE_DTYPE_UINT8`, or `LANCE_DTYPE_INT8`. Every normal nonzero lance-c return is converted to a Doris `Status`. This corroborates the report that the native-width query is intentional and that an ordinary Lance error would be handled without killing the BE. - In both lance-c v0.1.2 and v0.1.6, the `ffi_try!` helper only translates `Result::Err`; it does not catch Rust panics. More importantly, lance-c's release profile explicitly uses `panic = "abort"`. Therefore v0.1.6 fixes the reported Float16/Float64 cases through its newer Lance dependency, but it does **not** contain the broader panic class. - PR #66698 is open against `master` and currently changes the pin to v0.1.6, but its pushed diff does not add the required macOS IOKit framework. The v0.1.6 dependency lock includes `sysinfo -> objc2-io-kit`, which supports the reported `IORegistryEntry*` / `IOService*` linker failure and the need to validate the final BE link on both macOS architectures. - PR #66512 is a draft against `branch-4.1`. As of head `7380b3f2ff92996f13f36f1213966eb02e84642a`, its description documents the v0.1.6 and Float16/Float64 work, but the pushed tree still pins v0.1.2 and the committed generator/tests still create only Float32 indexed tables. The pushed contents need to be aligned with the description before this issue can rely on that PR for branch-4.1 coverage. ## Missing information / remaining uncertainty No additional information is required to accept and fix the reported crash. For an exact upstream root-cause attribution within lance-rs 4.0.1, however, please preserve one full Rust backtrace (`RUST_BACKTRACE=full`) and the exact Doris/lance-c commit SHAs used for the failing and passing runs. The current `arrow-array` downcast location proves a type-assumption panic but is not enough by itself to identify the precise Lance caller without guessing. The separately reported Int8-with-NULL and invalid hamming-index panics are credible indications of the same containment weakness, but they have different trigger paths and should not be assumed to share this Float16/Float64 root cause. ## Recommended next steps 1. Complete #66698 for `master`: add the macOS IOKit link dependency, record Linux plus macOS x86_64/arm64 build results, and run the Float16/Float64 IVF_FLAT queries with both indexed and `use_index=false` controls. 2. Update #66512 (or use a separate focused PR) for `branch-4.1` so the pushed change actually contains the v0.1.6 pin, IOKit handling, indexed Float16/Float64 fixtures, and regression assertions. The test must prove that an index was used, not merely that correct nearest rows were returned after a silent flat fallback. 3. Backport/land the dependency fix and coverage on both affected branches, then close this issue once the four primary cells pass: Float16 indexed/flat and Float64 indexed/flat. Keep Float32 indexed and UInt8/hamming as controls. 4. Track FFI panic containment separately in lance-c. A viable design must first use an unwind-capable release profile, then wrap exported ABI entry points (ideally through a common helper) with `catch_unwind` and translate panic payloads to `LanceErrorCode::Internal` / `lance_last_error_message()`. `catch_unwind` with the current `panic = "abort"` profile cannot work. Add an end-to-end test demonstrating that an induced Rust panic returns an error and leaves the hosting process alive, including panics arising during scanner execution. 5. File the Int8-with-NULL and invalid hamming-index triggers as separate bugs/tests. They should not block the targeted v0.1.6 fix, but they are useful acceptance cases for the containment work. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
