fix(rust/ffi): forward original vendor_code as an error detail on the 1.1.0 layout - #32
Open
fornwall wants to merge 1 commit into
Open
fix(rust/ffi): forward original vendor_code as an error detail on the 1.1.0 layout#32fornwall wants to merge 1 commit into
fornwall wants to merge 1 commit into
Conversation
… 1.1.0 layout The ADBC 1.1.0 error layout requires AdbcError.vendor_code to hold the ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA sentinel (INT32_MIN) whenever private_data carries the extended error info, so export_error unavoidably overwrites the driver's real vendor code (e.g. a gRPC status code) for 1.1.0-layout consumers. Only Rust-native and 1.0.0-layout callers could still see it. Preserve it instead as an additional error detail under the key "adbc.error.vendor_code" (decimal string, UTF-8), readable via AdbcErrorGetDetail. The detail is only synthesized when the code is meaningful (nonzero and not the sentinel), the 1.0.0 path is untouched, and the spec-mandated sentinel stays in place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bxmWF4Z9R4y1kaHEAPgS6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In the ADBC 1.1.0 C error layout,
AdbcError.vendor_codemust hold theADBC_ERROR_VENDOR_CODE_PRIVATE_DATAsentinel (INT32_MIN) to signal thatprivate_datacarries the extended error info. The Rust FFI exporter'sexport_errortherefore stamps the sentinel over the driver's real vendor code on the 1.1.0 path (rust/ffi/src/types.rs), destroying it for every 1.1.0-layout consumer — only Rust-native and 1.0.0-layout callers can still see it.This bit the adbc-spanner Rust driver (review finding UP-10): it keeps the numeric gRPC status code in
vendor_code(e.g.10= ABORTED) as a documented contract, but that contract silently does not hold for 1.1.0 C callers.Fix
When exporting through the 1.1.0 path and the Rust
Errorcarries a meaningful vendor code (nonzero and not the sentinel),export_errornow appends it as an additional error detail — the 1.1.0 rich-error-metadata mechanism, readable viaAdbcErrorGetDetail:adbc.error.vendor_code(exported asadbc_ffi::ERR_DETAIL_VENDOR_CODE) — namespacedadbc.<object>.<field>naming the exact spec field it mirrors, following the existing exporter-synthesized detail-key precedentadbc.drivermanager.driver_load_traceinrust/driver_manager/src/search.rsPG_DIAG_*details; no-binsuffix since it is not binary proto)The 1.0.0 path and the spec-mandated sentinel itself are unchanged; pre-existing details are preserved, the synthesized one is appended after them.
How the Go and C++ implementations handle the same collision
Neither forwards the vendor code as an error detail today, so the detail key introduced here has no cross-language precedent — but neither has a lossless answer either:
setErrWithDetailsin the generated exporters, templatego/adbc/pkg/_tmpl/driver.go.tmpl) makes a case-by-case trade:VendorCodeintovendor_code(same as Rust).vendor_codeandprivate_datais left unpopulated, sacrificing the extended layout to keep the code visible.VendorCodeis silently dropped.Status(c/driver/framework/status.h) carries a status code, message and details but has no vendor-code field at all, and nothing underc/driver/ever assigns a realvendor_code(the only occurrences are sentinel comparisons; likewise the legacyc/driver/common/utils.chelpers). C++-framework drivers simply never populate a vendor code.So this PR's behaviour is aligned with Go in intent (don't let the 1.1.0 sentinel destroy the code) but strictly more information-preserving in mechanism: the code survives even when other details are present (Go's lossy case), and the extended layout is never sacrificed (Go's no-details case). The key is deliberately language-neutral so the Go exporter could adopt the same detail to fix its lossy branch if this is proposed upstream.
Tests
test_set_error_out_extended_layout_forwards_vendor_code_as_detail— a 1.1.0 consumer reads the code back through the exporter's realErrorGetDetailCount/ErrorGetDetailfunctions (madepub(crate)for the test), alongside a preserved pre-existing detailtest_set_error_out_extended_layout_skips_meaningless_vendor_codes— no detail for0or the sentineltest_set_error_out_v100_layout_keeps_vendor_code_in_place— the 1.0.0 path still passes the code throughvendor_codeitselfcargo test -p adbc_ffi(11 passed),cargo clippy -p adbc_ffi --all-targets --all-features -- -Dwarningsandcargo fmt --all -- --checkare clean. Workspace clippy/test failures inadbc_driver_managerare pre-existing onmain(identical 29 dead-code errors with and without this change; they need the sqlite driver test lib).Upstreaming
This is a fork-internal staging of a fix intended to be proposed upstream to apache/arrow-adbc (see the Go/C++ comparison above — upstream could pair this with the matching fix to the Go exporter's lossy details-present branch).
🤖 Generated with Claude Code
https://claude.ai/code/session_013bxmWF4Z9R4y1kaHEAPgS6