Skip to content

feat(rust/ffi): report ECANCELED through exported streams for cancelled operations - #3

Open
fornwall wants to merge 1 commit into
mainfrom
rust-ffi-stream-errno-status
Open

feat(rust/ffi): report ECANCELED through exported streams for cancelled operations#3
fornwall wants to merge 1 commit into
mainfrom
rust-ffi-stream-errno-status

Conversation

@fornwall

@fornwall fornwall commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Fork-internal PR — intentionally targets fornwall/arrow-adbc:main, not upstream. Do not retarget to apache/arrow-adbc yet.

Problem

The driver exporter exports result readers via arrow-rs's FFI_ArrowArrayStream::new, whose error→errno mapping is hardcoded to the ArrowError variant: NotYetImplementedENOSYS, MemoryErrorENOMEM, IoErrorEIO, everything else→EINVAL. Rust ADBC drivers surface their rich adbc_core::error::Error through ArrowError::ExternalError, which lands in the EINVAL catch-all — so a driver whose in-flight query was cancelled has no way to report ECANCELED through the C stream, even though the ADBC spec (and the C++ validation suite's SqlQueryCancel) expect exactly that errno after AdbcStatementCancel.

Concretely: adbc-spanner's now-working cancellation fails SpannerStatementTest.SqlQueryCancel with errno 22 (EINVAL), message "Cancelled: operation cancelled" — the status is right there in the message but unreachable in the code.

Why owning the stream callbacks (and not something smaller)

This was checked against the C/C++ side of the repo, and the "invasive" part — implementing the ArrowArrayStream callbacks instead of delegating to arrow-rs — is exactly what the in-tree C/C++ drivers do. E.g. the PostgreSQL driver's TupleReader (c/driver/postgresql/statement.cc) implements GetSchema/GetNext by hand and returns InternalAdbcStatusCodeToErrno(status_); the shared status→errno table lives in c/driver/common/utils.c. arrow-rs's generic exporter is the outlier: it stringifies the error into its private data, so the ADBC status is unreachable once the stream is built.

Smaller alternatives considered and rejected:

  • Wrap the reader so errors map to a different ArrowError variant — no variant reaches ECANCELED; dead end.
  • Wrap the exported stream and remap EINVAL post-hoc — only the stringified message survives inside arrow-rs's private data, so this degenerates to sniffing "Cancelled:" prefixes; same amount of unsafe callback code, but fragile.
  • Upstream arrow-rs hook (e.g. FFI_ArrowArrayStream::new_with_error_code(reader, f)) — the genuinely smaller end-state, but needs an arrow-rs API addition and release; worth proposing separately.

Fix

New rust/ffi/src/exported_stream.rs: the same export machinery (adapted from arrow-rs's ffi_stream.rs, Apache-2.0), differing only in the error-code function:

  • the error source chain is searched for an adbc_core::error::Error;
  • a found ADBC status maps through errno_for_status, a direct port of the canonical InternalAdbcStatusCodeToErrno (c/driver/common/utils.c): CancelledECANCELED, NotFoundENOENT, NotImplementedENOTSUP, TimeoutETIMEDOUT, Unauthenticated/UnauthorizedEACCES, etc. — so Rust drivers report the same errnos through the C stream as the C/C++ drivers do;
  • errors without an ADBC status keep the arrow-rs variant mapping.

Errno constants come from libc (new workspace dependency, default-features = false) rather than a hand-maintained per-OS cfg table — platform-correct everywhere including Windows, mirroring how the C code gets them from <errno.h>.

All seven FFI_ArrowArrayStream::new(reader) export sites in driver_exporter.rs now go through export_reader.

Testing

Unit tests drive the exported stream through the raw C callbacks, exactly as a C consumer would:

  • end-of-stream returns 0 with a released array
  • a Cancelled ADBC error returns ECANCELED, message preserved via get_last_error
  • a Cancelled error nested deeper in a source chain is still found
  • the canonical table spot-checked across eight statuses
  • errors without an ADBC status keep the arrow-rs mapping (EINVAL/ENOSYS/EIO)
  • schema round-trips

cargo test -p adbc_ffi, cargo clippy -p adbc_ffi --all-targets -- -D warnings, cargo fmt --check all pass.

Follow-up (separate changes)

  • Bump the adbc_core/adbc_ffi git pin in adbc-spanner to include this, and re-gate SpannerStatementTest.SqlQueryCancel in its validation suite.
  • Propose an error-code hook upstream in arrow-rs, which would let this module shrink to a closure.
  • Potentially propose this upstream (apache/arrow-adbc) after it has proven out in the fork.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FXTDX6SvmGeXvttQux5zbS

@fornwall
fornwall force-pushed the rust-ffi-stream-errno-status branch 2 times, most recently from d91e7b2 to e20fa42 Compare July 8, 2026 00:30
The driver exporter exported result readers via arrow-rs's
FFI_ArrowArrayStream::new, which reduces every mid-stream failure to an
errno plus a message string: rich adbc_core errors surfaced through
ArrowError::ExternalError landed in the catch-all EINVAL, and the ADBC
1.1.0 ErrorFromArrayStream slot in the driver table was left as a TODO.

Replace the export machinery (adapted from arrow-rs's ffi_stream.rs)
with one that stashes the full adbc_core::error::Error found in the
error's source chain (or synthesized from the ArrowError variant):

- get_next/get_schema return the errno for the error's ADBC status via
  a port of the canonical InternalAdbcStatusCodeToErrno table, so e.g.
  cancelled operations report ECANCELED as the validation suite expects.
- ErrorFromArrayStream is now implemented, handing consumers the
  stream-owned FFI_AdbcError (message, SQLSTATE, vendor code, details)
  together with its status code, and NULL for foreign streams. Own
  streams are recognized via a registry of live private-data addresses,
  since Rust does not guarantee the function-pointer identity the C/C++
  drivers rely on (Miri fails such comparisons).

Tested by unit tests, a new end-to-end test driving the exported C
driver table against a failing dummy-driver stream, and a clean run of
the adbc_ffi suite under Miri.

Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
@fornwall
fornwall force-pushed the rust-ffi-stream-errno-status branch from e20fa42 to 3ec70f5 Compare July 8, 2026 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant