mbrobbel commented on code in PR #3381: URL: https://github.com/apache/arrow-adbc/pull/3381#discussion_r2312510322
########## rust/ffi/src/lib.rs: ########## @@ -0,0 +1,49 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! ADBC: Arrow Database Connectivity +//! +//! ADBC is a set of APIs and libraries for [Arrow](https://arrow.apache.org/)-native +//! access to databases. Execute SQL and [Substrait](https://substrait.io/) +//! queries, query database catalogs, and more, all using Arrow data to +//! eliminate unnecessary data copies, speed up access, and make it more +//! convenient to build analytical applications. +//! +//! Read more about ADBC at <https://arrow.apache.org/adbc/> +//! +//! This library currently provides: +//! - Structs for C-compatible items as defined in [`adbc.h`](https://github.com/apache/arrow-adbc/blob/main/c/include/arrow-adbc/adbc.h). +//! - A driver exporter that takes an implementation of the abstract API and +//! turns it into an object file that implements the C API. +//! +//! # Driver Exporter +//! +//! The driver exporter allows exposing native Rust drivers as C drivers to be +//! used by other languages via their own driver manager. Once you have an +//! implementation of [adbc_core::Driver], provided that it also implements [Default], you +//! can build it as an object file implementing the C API with the +//! [export_driver] macro. + +pub mod driver_exporter; +#[doc(hidden)] +pub use driver_exporter::FFIDriver; +pub mod methods; +pub(crate) mod types; +pub use types::{ + FFI_AdbcConnection, FFI_AdbcDatabase, FFI_AdbcDriver, FFI_AdbcDriverInitFunc, FFI_AdbcError, + FFI_AdbcErrorDetail, FFI_AdbcPartitions, FFI_AdbcStatement, +}; Review Comment: Now that these types are in a different crate, we can maybe remove the `FFI_`/`FFI` prefix? ########## rust/core/src/options.rs: ########## @@ -37,7 +37,7 @@ pub enum OptionValue { impl OptionValue { /// Gets the data type of the option's value. - pub(crate) fn get_type(&self) -> &str { + pub fn get_type(&self) -> &str { Review Comment: If we are marking this `pub` maybe we should consider renaming this? ########## rust/driver/datafusion/Cargo.toml: ########## @@ -48,4 +49,4 @@ crate-type = ["lib", "cdylib"] [features] default = [] -ffi = [] +ffi = ["adbc_ffi"] Review Comment: ```suggestion ffi = ["dep:adbc_ffi"] ``` ########## rust/core/src/error.rs: ########## @@ -153,3 +157,59 @@ impl From<std::ffi::IntoStringError> for Error { error.into() } } + +impl TryFrom<AdbcStatusCode> for Status { + type Error = Error; + + fn try_from(value: AdbcStatusCode) -> Result<Self> { + match value { + constants::ADBC_STATUS_OK => Ok(Status::Ok), + constants::ADBC_STATUS_UNKNOWN => Ok(Status::Unknown), + constants::ADBC_STATUS_NOT_IMPLEMENTED => Ok(Status::NotImplemented), + constants::ADBC_STATUS_NOT_FOUND => Ok(Status::NotFound), + constants::ADBC_STATUS_ALREADY_EXISTS => Ok(Status::AlreadyExists), + constants::ADBC_STATUS_INVALID_ARGUMENT => Ok(Status::InvalidArguments), + constants::ADBC_STATUS_INVALID_STATE => Ok(Status::InvalidState), + constants::ADBC_STATUS_INVALID_DATA => Ok(Status::InvalidData), + constants::ADBC_STATUS_INTEGRITY => Ok(Status::Integrity), + constants::ADBC_STATUS_INTERNAL => Ok(Status::Internal), + constants::ADBC_STATUS_IO => Ok(Status::IO), + constants::ADBC_STATUS_CANCELLED => Ok(Status::Cancelled), + constants::ADBC_STATUS_TIMEOUT => Ok(Status::Timeout), + constants::ADBC_STATUS_UNAUTHENTICATED => Ok(Status::Unauthenticated), + constants::ADBC_STATUS_UNAUTHORIZED => Ok(Status::Unauthorized), + v => Err(Error::with_message_and_status( + format!("Unknown status code: {v}"), + Status::InvalidData, + )), + } + } +} + +impl From<Status> for AdbcStatusCode { + fn from(value: Status) -> Self { + match value { + Status::Ok => constants::ADBC_STATUS_OK, + Status::Unknown => constants::ADBC_STATUS_UNKNOWN, + Status::NotImplemented => constants::ADBC_STATUS_NOT_IMPLEMENTED, + Status::NotFound => constants::ADBC_STATUS_NOT_FOUND, + Status::AlreadyExists => constants::ADBC_STATUS_ALREADY_EXISTS, + Status::InvalidArguments => constants::ADBC_STATUS_INVALID_ARGUMENT, + Status::InvalidState => constants::ADBC_STATUS_INVALID_STATE, + Status::InvalidData => constants::ADBC_STATUS_INVALID_DATA, + Status::Integrity => constants::ADBC_STATUS_INTEGRITY, + Status::Internal => constants::ADBC_STATUS_INTERNAL, + Status::IO => constants::ADBC_STATUS_IO, + Status::Cancelled => constants::ADBC_STATUS_CANCELLED, + Status::Timeout => constants::ADBC_STATUS_TIMEOUT, + Status::Unauthenticated => constants::ADBC_STATUS_UNAUTHENTICATED, + Status::Unauthorized => constants::ADBC_STATUS_UNAUTHORIZED, + } + } +} + +impl From<&Status> for AdbcStatusCode { + fn from(value: &Status) -> Self { + (*value).into() + } +} Review Comment: `Status` is `Copy` so I don't think we need this. ```suggestion ``` ########## rust/core/src/error.rs: ########## @@ -22,6 +22,10 @@ use std::{ffi::NulError, fmt::Display}; use arrow_schema::ArrowError; +use crate::constants; + +pub type AdbcStatusCode = u8; Review Comment: Perhaps we can rename this to just `StatusCode`? -- 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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
