From 97999a45e6971ee3f0a7190eae6717eb3a3db888 Mon Sep 17 00:00:00 2001 From: Fredrik Fornwall Date: Mon, 13 Jul 2026 23:50:30 +0200 Subject: [PATCH] fix(rust/core): add InfoCode::Other to handle arbitrary codes `InfoCode` was a closed set of the 11 standard codes, so requests for XDBC-range ([500, 1_000)) or vendor-specific (>= 10_000) info codes could not be represented: the FFI exporter errored on unknown values before the wrapped Rust driver was called, and the JavaScript client dropped them before the driver manager forwarded the request to the loaded driver. A driver could still emit such codes when asked for everything (`info_codes` == `None`), so a vendor code would appear in "fetch all" results but could not be requested explicitly. Add an `InfoCode::Other(u32)` catch-all variant (mirroring the existing `Other` variants on `OptionDatabase`/`OptionConnection`/`OptionStatement`) so codes survive the u32 round-trip, and pass requested codes through unfiltered in both the FFI exporter and the JavaScript client, leaving "ignore unrecognized codes" to the driver as adbc.h states: > Drivers/vendors will ignore requests for unrecognized codes > (the row will be omitted from the result) Signed-off-by: Fredrik Fornwall --- javascript/src/client.rs | 2 +- rust/core/src/options.rs | 64 ++++++++---- rust/driver/dummy/src/lib.rs | 46 ++++++--- .../dummy/tests/driver_exporter_dummy.rs | 97 ++++++++++++++++++- rust/ffi/src/driver_exporter.rs | 5 +- 5 files changed, 175 insertions(+), 39 deletions(-) diff --git a/javascript/src/client.rs b/javascript/src/client.rs index 6324131688..dee9af31a1 100644 --- a/javascript/src/client.rs +++ b/javascript/src/client.rs @@ -230,7 +230,7 @@ impl AdbcConnectionCore { .map_err(|e| ClientError::Other(e.to_string()))?; let codes: Option> = info_codes.map(|v| { v.into_iter() - .filter_map(|code| InfoCode::try_from(code).ok()) + .map(InfoCode::from) .collect::>() }); let reader = conn.get_info(codes)?; diff --git a/rust/core/src/options.rs b/rust/core/src/options.rs index 99fdb59078..dd0e015757 100644 --- a/rust/core/src/options.rs +++ b/rust/core/src/options.rs @@ -223,6 +223,19 @@ pub enum InfoCode { /// /// ADBC API revision 1.1.0 DriverAdbcVersion, + /// Any other info code than the ones listed above. The value is the raw code. + /// + /// Codes `[0, 10_000)` are reserved for ADBC usage (of which `[500, 1_000)` + /// is reserved for "XDBC" information since ADBC API revision 1.1.0), while + /// codes `10_000` and higher are driver/vendor-specific. Drivers ignore + /// requests for codes they don't recognize (the row is omitted from the + /// result). + /// + /// Note: to keep equality and hashing consistent, this variant should never + /// be constructed with the value of a code that has a dedicated variant; + /// use [InfoCode::from] to convert from a raw `u32`, which only produces + /// `Other` for values without one. + Other(u32), } impl From<&InfoCode> for u32 { @@ -243,34 +256,30 @@ impl From<&InfoCode> for u32 { InfoCode::DriverVersion => constants::ADBC_INFO_DRIVER_VERSION, InfoCode::DriverArrowVersion => constants::ADBC_INFO_DRIVER_ARROW_VERSION, InfoCode::DriverAdbcVersion => constants::ADBC_INFO_DRIVER_ADBC_VERSION, + InfoCode::Other(v) => *v, } } } -impl TryFrom for InfoCode { - type Error = Error; - - fn try_from(value: u32) -> Result { +impl From for InfoCode { + fn from(value: u32) -> Self { match value { - constants::ADBC_INFO_VENDOR_NAME => Ok(InfoCode::VendorName), - constants::ADBC_INFO_VENDOR_VERSION => Ok(InfoCode::VendorVersion), - constants::ADBC_INFO_VENDOR_ARROW_VERSION => Ok(InfoCode::VendorArrowVersion), - constants::ADBC_INFO_VENDOR_SQL => Ok(InfoCode::VendorSql), - constants::ADBC_INFO_VENDOR_SUBSTRAIT => Ok(InfoCode::VendorSubstrait), + constants::ADBC_INFO_VENDOR_NAME => InfoCode::VendorName, + constants::ADBC_INFO_VENDOR_VERSION => InfoCode::VendorVersion, + constants::ADBC_INFO_VENDOR_ARROW_VERSION => InfoCode::VendorArrowVersion, + constants::ADBC_INFO_VENDOR_SQL => InfoCode::VendorSql, + constants::ADBC_INFO_VENDOR_SUBSTRAIT => InfoCode::VendorSubstrait, constants::ADBC_INFO_VENDOR_SUBSTRAIT_MIN_VERSION => { - Ok(InfoCode::VendorSubstraitMinVersion) + InfoCode::VendorSubstraitMinVersion } constants::ADBC_INFO_VENDOR_SUBSTRAIT_MAX_VERSION => { - Ok(InfoCode::VendorSubstraitMaxVersion) + InfoCode::VendorSubstraitMaxVersion } - constants::ADBC_INFO_DRIVER_NAME => Ok(InfoCode::DriverName), - constants::ADBC_INFO_DRIVER_VERSION => Ok(InfoCode::DriverVersion), - constants::ADBC_INFO_DRIVER_ARROW_VERSION => Ok(InfoCode::DriverArrowVersion), - constants::ADBC_INFO_DRIVER_ADBC_VERSION => Ok(InfoCode::DriverAdbcVersion), - v => Err(Error::with_message_and_status( - format!("Unknown info code: {v}"), - Status::InvalidData, - )), + constants::ADBC_INFO_DRIVER_NAME => InfoCode::DriverName, + constants::ADBC_INFO_DRIVER_VERSION => InfoCode::DriverVersion, + constants::ADBC_INFO_DRIVER_ARROW_VERSION => InfoCode::DriverArrowVersion, + constants::ADBC_INFO_DRIVER_ADBC_VERSION => InfoCode::DriverAdbcVersion, + v => InfoCode::Other(v), } } } @@ -741,3 +750,20 @@ impl std::fmt::Display for Statistics { write!(f, "{}", self.as_ref()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn info_code_u32_roundtrip() { + for code in (0..20_000_u32).chain([u32::MAX]) { + assert_eq!(u32::from(&InfoCode::from(code)), code); + } + assert_eq!( + InfoCode::from(constants::ADBC_INFO_VENDOR_NAME), + InfoCode::VendorName + ); + assert_eq!(InfoCode::from(10_042), InfoCode::Other(10_042)); + } +} diff --git a/rust/driver/dummy/src/lib.rs b/rust/driver/dummy/src/lib.rs index 7879873ca9..8c3b92e762 100644 --- a/rust/driver/dummy/src/lib.rs +++ b/rust/driver/dummy/src/lib.rs @@ -37,6 +37,8 @@ use adbc_core::{ schemas, }; +pub const VENDOR_INFO_CODE: u32 = 10_042; + #[derive(Debug)] pub struct SingleBatchReader { batch: Option, @@ -312,9 +314,9 @@ impl Connection for DummyConnection { fn get_info( &self, - _codes: Option>, + codes: Option>, ) -> Result> { - let string_value_array = StringArray::from(vec!["MyVendorName"]); + let string_value_array = StringArray::from(vec!["MyVendorName", "MyVendorInfoValue"]); let bool_value_array = BooleanArray::from(vec![true]); let int64_value_array = Int64Array::from(vec![42]); let int32_bitmask_array = Int32Array::from(vec![1337]); @@ -355,20 +357,34 @@ impl Connection for DummyConnection { false, )?; - let name_array = UInt32Array::from(vec![ - Into::::into(&InfoCode::VendorName), - Into::::into(&InfoCode::VendorVersion), - Into::::into(&InfoCode::VendorArrowVersion), - Into::::into(&InfoCode::DriverName), - Into::::into(&InfoCode::DriverVersion), - Into::::into(&InfoCode::DriverArrowVersion), - ]); - - let type_id_buffer = [0_i8, 1, 2, 3, 4, 5] - .into_iter() + // Every info value this driver knows, as (code, union type id, offset into that type's + // child array). Includes a vendor-specific code to exercise `InfoCode::Other`. + let known_info: [(InfoCode, i8, i32); 7] = [ + (InfoCode::VendorName, 0, 0), + (InfoCode::VendorVersion, 1, 0), + (InfoCode::VendorArrowVersion, 2, 0), + (InfoCode::DriverName, 3, 0), + (InfoCode::DriverVersion, 4, 0), + (InfoCode::DriverArrowVersion, 5, 0), + (InfoCode::Other(VENDOR_INFO_CODE), 0, 1), + ]; + let rows: Vec<&(InfoCode, i8, i32)> = known_info + .iter() + .filter(|(code, _, _)| codes.as_ref().is_none_or(|codes| codes.contains(code))) + .collect(); + + let name_array = UInt32Array::from( + rows.iter() + .map(|(code, _, _)| code.into()) + .collect::>(), + ); + let type_id_buffer = rows + .iter() + .map(|(_, type_id, _)| *type_id) .collect::>(); - let value_offsets_buffer = [0_i32, 0, 0, 0, 0, 0] - .into_iter() + let value_offsets_buffer = rows + .iter() + .map(|(_, _, offset)| *offset) .collect::>(); let value_array = UnionArray::try_new( diff --git a/rust/driver/dummy/tests/driver_exporter_dummy.rs b/rust/driver/dummy/tests/driver_exporter_dummy.rs index b9bc989416..36e7657f92 100644 --- a/rust/driver/dummy/tests/driver_exporter_dummy.rs +++ b/rust/driver/dummy/tests/driver_exporter_dummy.rs @@ -19,11 +19,14 @@ /// directly using the Rust API (native) and through the exported driver via the /// driver manager (exported). That allows us to test that data correctly round-trip /// between C and Rust. +use std::collections::HashSet; use std::ffi::CString; use std::ops::Deref; use std::sync::Arc; -use arrow_array::ffi_stream::FFI_ArrowArrayStream; +use arrow_array::cast::AsArray; +use arrow_array::ffi_stream::{ArrowArrayStreamReader, FFI_ArrowArrayStream}; +use arrow_array::types::UInt32Type; use arrow_array::{Array, Float64Array, Int64Array, RecordBatch, RecordBatchReader, StringArray}; use arrow_schema::{DataType, Field, Schema}; use arrow_select::concat::concat_batches; @@ -365,7 +368,99 @@ fn test_connection_get_info() { .unwrap(), ); assert_eq!(exported_info.schema(), *schemas::GET_INFO_SCHEMA.deref()); + // The dummy driver recognizes `DriverName` but not `DriverAdbcVersion`, whose row is omitted. + assert_eq!(exported_info.num_rows(), 1); assert_eq!(exported_info, native_info); + + let vendor_codes: HashSet = [InfoCode::Other(adbc_dummy::VENDOR_INFO_CODE)].into(); + let exported_info = concat_reader( + exported_connection + .get_info(Some(vendor_codes.clone())) + .unwrap(), + ); + let native_info = concat_reader(native_connection.get_info(Some(vendor_codes)).unwrap()); + assert_eq!(exported_info.num_rows(), 1); + assert_eq!( + exported_info + .column(0) + .as_primitive::() + .value(0), + adbc_dummy::VENDOR_INFO_CODE + ); + assert_eq!(exported_info, native_info); +} + +// Driven at the C ABI (unlike the other tests) to exercise the exporter's raw `u32` code +// handling: codes without a dedicated `InfoCode` variant must reach the driver as +// `InfoCode::Other` (so it can answer vendor-specific codes), and per the ADBC spec the driver +// ignores requests for codes it doesn't recognize (the row is omitted) rather than failing the +// call. +#[test] +fn test_connection_get_info_ignores_unrecognized_codes() { + let driver = DummyDriver::ffi_driver(); + let mut error = FFI_AdbcError::default(); + let err = &mut error as *mut FFI_AdbcError; + + unsafe { + let mut database = FFI_AdbcDatabase::default(); + assert_eq!( + driver.DatabaseNew.unwrap()(&mut database, err), + ADBC_STATUS_OK + ); + assert_eq!( + driver.DatabaseInit.unwrap()(&mut database, err), + ADBC_STATUS_OK + ); + + let mut connection = FFI_AdbcConnection::default(); + assert_eq!( + driver.ConnectionNew.unwrap()(&mut connection, err), + ADBC_STATUS_OK + ); + assert_eq!( + driver.ConnectionInit.unwrap()(&mut connection, &mut database, err), + ADBC_STATUS_OK + ); + + let info_codes: [u32; 3] = [ + Into::::into(&InfoCode::DriverName), + adbc_dummy::VENDOR_INFO_CODE, + u32::MAX, + ]; + let mut stream = FFI_ArrowArrayStream::empty(); + assert_eq!( + driver.ConnectionGetInfo.unwrap()( + &mut connection, + info_codes.as_ptr(), + info_codes.len(), + &mut stream, + err, + ), + ADBC_STATUS_OK, + "an unrecognized info code must be ignored, not fail the call" + ); + + let info = concat_reader(ArrowArrayStreamReader::try_new(stream).unwrap()); + let returned_codes: HashSet = info + .column(0) + .as_primitive::() + .values() + .iter() + .copied() + .collect(); + assert_eq!( + returned_codes, + [ + Into::::into(&InfoCode::DriverName), + adbc_dummy::VENDOR_INFO_CODE + ] + .into(), + "the vendor-specific code must be answered and the unrecognized one omitted" + ); + + driver.ConnectionRelease.unwrap()(&mut connection, err); + driver.DatabaseRelease.unwrap()(&mut database, err); + } } #[test] diff --git a/rust/ffi/src/driver_exporter.rs b/rust/ffi/src/driver_exporter.rs index ca483c5a42..429f0caf31 100644 --- a/rust/ffi/src/driver_exporter.rs +++ b/rust/ffi/src/driver_exporter.rs @@ -1163,9 +1163,8 @@ extern "C" fn connection_get_info( None } else { let info_codes = unsafe { std::slice::from_raw_parts(info_codes, info_codes_length) }; - let info_codes: Result> = - info_codes.iter().map(|c| InfoCode::try_from(*c)).collect(); - let info_codes = check_err!(info_codes, error); + let info_codes: HashSet = + info_codes.iter().map(|c| InfoCode::from(*c)).collect(); Some(info_codes) };