alexandreyc commented on code in PR #1725: URL: https://github.com/apache/arrow-adbc/pull/1725#discussion_r1568395445
########## rust2/core/src/options.rs: ########## @@ -0,0 +1,492 @@ +// 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. + +//! Various option and configuration types. + +use std::os::raw::c_int; + +use crate::{ + error::{Error, Status}, + ffi::constants, +}; + +/// Option value. +/// +/// Can be created with various implementations of [From]. +#[derive(Debug)] +pub enum OptionValue { + String(String), + Bytes(Vec<u8>), + Int(i64), + Double(f64), +} + +impl From<String> for OptionValue { + fn from(value: String) -> Self { + Self::String(value) + } +} + +impl From<&str> for OptionValue { + fn from(value: &str) -> Self { + Self::String(value.into()) + } +} + +impl From<i64> for OptionValue { + fn from(value: i64) -> Self { + Self::Int(value) + } +} + +impl From<f64> for OptionValue { + fn from(value: f64) -> Self { + Self::Double(value) + } +} + +impl From<Vec<u8>> for OptionValue { + fn from(value: Vec<u8>) -> Self { + Self::Bytes(value) + } +} + +impl From<&[u8]> for OptionValue { + fn from(value: &[u8]) -> Self { + Self::Bytes(value.into()) + } +} + +impl<const N: usize> From<[u8; N]> for OptionValue { + fn from(value: [u8; N]) -> Self { + Self::Bytes(value.into()) + } +} + +impl<const N: usize> From<&[u8; N]> for OptionValue { + fn from(value: &[u8; N]) -> Self { + Self::Bytes(value.into()) + } +} + +/// ADBC revision versions. +#[derive(Clone, Copy)] +pub enum AdbcVersion { + /// Version 1.0.0. + V100, + /// Version 1.1.0. + V110, +} + +impl From<AdbcVersion> for c_int { + fn from(value: AdbcVersion) -> Self { + match value { + AdbcVersion::V100 => constants::ADBC_VERSION_1_0_0, + AdbcVersion::V110 => constants::ADBC_VERSION_1_1_0, + } + } +} + +/// Info codes for database/driver metadata. +#[derive(Debug)] +pub enum InfoCode { + /// The database vendor/product name (type: utf8). + VendorName, + /// The database vendor/product version (type: utf8). + VendorVersion, + /// The database vendor/product Arrow library version (type: utf8). + VendorArrowVersion, + /// The driver name (type: utf8). + DriverName, + /// The driver version (type: utf8). + DriverVersion, + /// The driver Arrow library version (type: utf8). + DriverArrowVersion, + /// The driver ADBC API version (type: int64). + /// + /// # Since + /// + /// ADBC API revision 1.1.0 + DriverAdbcVersion, + // TODO(alexandreyc): add new codes (see https://github.com/apache/arrow-adbc/commit/aa04aadccd319e6fa3abb07154fa8d87b58d5c21) +} + +impl From<&InfoCode> for u32 { + fn from(value: &InfoCode) -> Self { + match value { + InfoCode::VendorName => constants::ADBC_INFO_VENDOR_NAME, + InfoCode::VendorVersion => constants::ADBC_INFO_VENDOR_VERSION, + InfoCode::VendorArrowVersion => constants::ADBC_INFO_VENDOR_ARROW_VERSION, + InfoCode::DriverName => constants::ADBC_INFO_DRIVER_NAME, + InfoCode::DriverVersion => constants::ADBC_INFO_DRIVER_VERSION, + InfoCode::DriverArrowVersion => constants::ADBC_INFO_DRIVER_ARROW_VERSION, + InfoCode::DriverAdbcVersion => constants::ADBC_INFO_DRIVER_ADBC_VERSION, + } + } +} + +impl TryFrom<u32> for InfoCode { + type Error = Error; + + fn try_from(value: u32) -> Result<Self, Self::Error> { + 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_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, + )), + } + } +} + +/// Depth parameter for [get_objects][crate::Connection::get_objects] method. +#[derive(Debug)] +pub enum ObjectDepth { Review Comment: I've chosen to not rely on integer representation of enums (and instead write explicit TryFrom/From) because of this: ```rust use num_enum::{FromPrimitive, IntoPrimitive}; #[repr(u32)] #[derive(FromPrimitive, IntoPrimitive)] pub enum InfoCode { VendorName = 0, VendorVersion = 1, VendorArrowVersion = 2, DriverName = 100, DriverVersion = 101, DriverArrowVersion = 102, #[num_enum(catch_all)] Other(u32), } fn main() { println!("size = {}", std::mem::size_of::<InfoCode>()); // size = 8 // not u32! } ``` This snippet of code is present is the current version (the one in `/rust`). When crossing FFI boundaries by passing for instance an `&[InfoCode]` it introduces a memory corruption... So, I we stay away from `#[num_enum(catch_all)]` I'm OK to rely on `#[repr(integer)]` instead of defining constants and conversion traits. -- 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
