refactor(odbc): fix OdbcRow and OdbcValue to contain correctly decoded data types

- Replaced the previous tuple structure in OdbcRow with a more comprehensive OdbcValue struct to encapsulate various data types and nullability.
- Updated methods in OdbcRow to utilize the new OdbcValue structure, simplifying value retrieval.
- Refactored data extraction functions in the connection worker to support the new OdbcValue format, enhancing type safety and clarity.
- Adjusted tests to validate the new data handling logic and ensure compatibility across different database types.

Author: lovasoa

sqlx-core/src/odbc/connection/worker.rs

@@ -16,7 +16,7 @@ use either::Either;
 #[allow(unused_imports)]
 use odbc_api::handles::Statement as OdbcStatementTrait;
 use odbc_api::handles::StatementImpl;
-use odbc_api::{Cursor, CursorRow, IntoParameter, Preallocated, ResultSetMetadata};
+use odbc_api::{Cursor, CursorRow, IntoParameter, Nullable, Preallocated, ResultSetMetadata};
 
 // Type aliases for commonly used types
 type OdbcConnection = odbc_api::Connection<'static>;
@@ -562,7 +562,7 @@ where
     OdbcColumn {
         name: decode_column_name(cd.name, index),
         type_info: OdbcTypeInfo::new(cd.data_type),
-        ordinal: (index - 1) as usize,
+        ordinal: usize::from(index.checked_sub(1).unwrap()),
     }
 }
 
@@ -581,7 +581,7 @@ where
         let values = collect_row_values(&mut row, columns)?;
         let row_data = OdbcRow {
             columns: columns.to_vec(),
-            values,
+            values: values.into_iter().map(|(_, value)| value).collect(),
         };
 
         if send_row(tx, row_data).is_err() {
@@ -601,7 +601,7 @@ where
 fn collect_row_values(
     row: &mut CursorRow<'_>,
     columns: &[OdbcColumn],
-) -> Result<Vec<(OdbcTypeInfo, Option<Vec<u8>>)>, Error> {
+) -> Result<Vec<(OdbcTypeInfo, crate::odbc::OdbcValue)>, Error> {
     columns
         .iter()
         .enumerate()
@@ -613,37 +613,155 @@ fn collect_column_value(
     row: &mut CursorRow<'_>,
     index: usize,
     column: &OdbcColumn,
-) -> Result<(OdbcTypeInfo, Option<Vec<u8>>), Error> {
+) -> Result<(OdbcTypeInfo, crate::odbc::OdbcValue), Error> {
+    use odbc_api::DataType;
+
     let col_idx = (index + 1) as u16;
+    let type_info = column.type_info.clone();
+    let data_type = type_info.data_type();
+
+    // Extract value based on data type
+    let value = match data_type {
+        // Integer types
+        DataType::TinyInt
+        | DataType::SmallInt
+        | DataType::Integer
+        | DataType::BigInt
+        | DataType::Bit => extract_int(row, col_idx, &type_info)?,
+
+        // Floating point types
+        DataType::Real => extract_float::<f32>(row, col_idx, &type_info)?,
+        DataType::Float { .. } | DataType::Double => {
+            extract_float::<f64>(row, col_idx, &type_info)?
+        }
+
+        // String types
+        DataType::Char { .. }
+        | DataType::Varchar { .. }
+        | DataType::LongVarchar { .. }
+        | DataType::WChar { .. }
+        | DataType::WVarchar { .. }
+        | DataType::WLongVarchar { .. }
+        | DataType::Date
+        | DataType::Time { .. }
+        | DataType::Timestamp { .. }
+        | DataType::Decimal { .. }
+        | DataType::Numeric { .. } => extract_text(row, col_idx, &type_info)?,
+
+        // Binary types
+        DataType::Binary { .. } | DataType::Varbinary { .. } | DataType::LongVarbinary { .. } => {
+            extract_binary(row, col_idx, &type_info)?
+        }
 
-    // Try text first
-    match try_get_text(row, col_idx) {
-        Ok(value) => Ok((column.type_info.clone(), value)),
-        Err(_) => {
-            // Fall back to binary
-            match try_get_binary(row, col_idx) {
-                Ok(value) => Ok((column.type_info.clone(), value)),
-                Err(e) => Err(Error::from(e)),
+        // Unknown types - try text first, fall back to binary
+        DataType::Unknown | DataType::Other { .. } => {
+            match extract_text(row, col_idx, &type_info) {
+                Ok(v) => v,
+                Err(_) => extract_binary(row, col_idx, &type_info)?,
             }
         }
-    }
+    };
+
+    Ok((type_info, value))
+}
+
+fn extract_int(
+    row: &mut CursorRow<'_>,
+    col_idx: u16,
+    type_info: &OdbcTypeInfo,
+) -> Result<crate::odbc::OdbcValue, Error> {
+    let mut nullable = Nullable::<i64>::null();
+    row.get_data(col_idx, &mut nullable)?;
+
+    let (is_null, int) = match nullable.into_opt() {
+        None => (true, None),
+        Some(v) => (false, Some(v.into())),
+    };
+
+    Ok(crate::odbc::OdbcValue {
+        type_info: type_info.clone(),
+        is_null,
+        text: None,
+        blob: None,
+        int,
+        float: None,
+    })
+}
+
+fn extract_float<T>(
+    row: &mut CursorRow<'_>,
+    col_idx: u16,
+    type_info: &OdbcTypeInfo,
+) -> Result<crate::odbc::OdbcValue, Error>
+where
+    T: Into<f64> + Default,
+    odbc_api::Nullable<T>: odbc_api::parameter::CElement + odbc_api::handles::CDataMut,
+{
+    let mut nullable = Nullable::<T>::null();
+    row.get_data(col_idx, &mut nullable)?;
+
+    let (is_null, float) = match nullable.into_opt() {
+        None => (true, None),
+        Some(v) => (false, Some(v.into())),
+    };
+
+    Ok(crate::odbc::OdbcValue {
+        type_info: type_info.clone(),
+        is_null,
+        text: None,
+        blob: None,
+        int: None,
+        float,
+    })
 }
 
-fn try_get_text(row: &mut CursorRow<'_>, col_idx: u16) -> Result<Option<Vec<u8>>, odbc_api::Error> {
+fn extract_text(
+    row: &mut CursorRow<'_>,
+    col_idx: u16,
+    type_info: &OdbcTypeInfo,
+) -> Result<crate::odbc::OdbcValue, Error> {
     let mut buf = Vec::new();
-    match row.get_text(col_idx, &mut buf)? {
-        true => Ok(Some(buf)),
-        false => Ok(None),
-    }
+    let is_some = row.get_text(col_idx, &mut buf)?;
+
+    let (is_null, text) = if !is_some {
+        (true, None)
+    } else {
+        match String::from_utf8(buf) {
+            Ok(s) => (false, Some(s)),
+            Err(e) => return Err(Error::Decode(e.into())),
+        }
+    };
+
+    Ok(crate::odbc::OdbcValue {
+        type_info: type_info.clone(),
+        is_null,
+        text,
+        blob: None,
+        int: None,
+        float: None,
+    })
 }
 
-fn try_get_binary(
+fn extract_binary(
     row: &mut CursorRow<'_>,
     col_idx: u16,
-) -> Result<Option<Vec<u8>>, odbc_api::Error> {
+    type_info: &OdbcTypeInfo,
+) -> Result<crate::odbc::OdbcValue, Error> {
     let mut buf = Vec::new();
-    match row.get_binary(col_idx, &mut buf)? {
-        true => Ok(Some(buf)),
-        false => Ok(None),
-    }
+    let is_some = row.get_binary(col_idx, &mut buf)?;
+
+    let (is_null, blob) = if !is_some {
+        (true, None)
+    } else {
+        (false, Some(buf))
+    };
+
+    Ok(crate::odbc::OdbcValue {
+        type_info: type_info.clone(),
+        is_null,
+        text: None,
+        blob,
+        int: None,
+        float: None,
+    })
 }

sqlx-core/src/odbc/row.rs

@@ -1,13 +1,14 @@
 use crate::column::ColumnIndex;
 use crate::database::HasValueRef;
 use crate::error::Error;
-use crate::odbc::{Odbc, OdbcColumn, OdbcTypeInfo, OdbcValueRef};
+use crate::odbc::{Odbc, OdbcColumn, OdbcValue};
 use crate::row::Row;
+use crate::value::Value;
 
 #[derive(Debug, Clone)]
 pub struct OdbcRow {
     pub(crate) columns: Vec<OdbcColumn>,
-    pub(crate) values: Vec<(OdbcTypeInfo, Option<Vec<u8>>)>,
+    pub(crate) values: Vec<OdbcValue>,
 }
 
 impl Row for OdbcRow {
@@ -25,15 +26,8 @@ impl Row for OdbcRow {
         I: ColumnIndex<Self>,
     {
         let idx = index.index(self)?;
-        let (ti, data) = &self.values[idx];
-        Ok(OdbcValueRef {
-            type_info: ti.clone(),
-            is_null: data.is_none(),
-            text: None,
-            blob: data.as_deref(),
-            int: None,
-            float: None,
-        })
+        let value = &self.values[idx];
+        Ok(value.as_ref())
     }
 }
 
@@ -66,6 +60,8 @@ mod tests {
     use odbc_api::DataType;
 
     fn create_test_row() -> OdbcRow {
+        use crate::odbc::OdbcValue;
+
         OdbcRow {
             columns: vec![
                 OdbcColumn {
@@ -85,15 +81,30 @@ mod tests {
                 },
             ],
             values: vec![
-                (OdbcTypeInfo::new(DataType::Integer), Some(vec![1, 2, 3, 4])),
-                (
-                    OdbcTypeInfo::new(DataType::Varchar { length: None }),
-                    Some(b"test".to_vec()),
-                ),
-                (
-                    OdbcTypeInfo::new(DataType::Double),
-                    Some(vec![1, 2, 3, 4, 5, 6, 7, 8]),
-                ),
+                OdbcValue {
+                    type_info: OdbcTypeInfo::new(DataType::Integer),
+                    is_null: false,
+                    text: None,
+                    blob: None,
+                    int: Some(42),
+                    float: None,
+                },
+                OdbcValue {
+                    type_info: OdbcTypeInfo::new(DataType::Varchar { length: None }),
+                    is_null: false,
+                    text: Some("test".to_string()),
+                    blob: None,
+                    int: None,
+                    float: None,
+                },
+                OdbcValue {
+                    type_info: OdbcTypeInfo::new(DataType::Double),
+                    is_null: false,
+                    text: None,
+                    blob: None,
+                    int: None,
+                    float: Some(std::f64::consts::PI),
+                },
             ],
         }
     }

sqlx-core/src/odbc/types/bigdecimal.rs

@@ -3,7 +3,7 @@ use crate::encode::Encode;
 use crate::error::BoxDynError;
 use crate::odbc::{DataTypeExt, Odbc, OdbcArgumentValue, OdbcTypeInfo, OdbcValueRef};
 use crate::types::Type;
-use bigdecimal::BigDecimal;
+use bigdecimal::{BigDecimal, FromPrimitive};
 use odbc_api::DataType;
 use std::str::FromStr;
 
@@ -36,7 +36,19 @@ impl<'q> Encode<'q, Odbc> for BigDecimal {
 
 impl<'r> Decode<'r, Odbc> for BigDecimal {
     fn decode(value: OdbcValueRef<'r>) -> Result<Self, BoxDynError> {
-        let s = <String as Decode<'r, Odbc>>::decode(value)?;
-        Ok(BigDecimal::from_str(&s)?)
+        if let Some(int) = value.int {
+            return Ok(BigDecimal::from(int));
+        }
+        if let Some(float) = value.float {
+            return Ok(BigDecimal::from_f64(float).ok_or(format!("bad float: {}", float))?);
+        }
+        if let Some(text) = value.text {
+            return Ok(BigDecimal::from_str(&text).map_err(|e| format!("bad decimal text: {}", e))?);
+        }
+        if let Some(bytes) = value.blob {
+            return Ok(BigDecimal::parse_bytes(bytes, 10)
+                .ok_or(format!("bad base10 bytes: {:?}", bytes))?);
+        }
+        Err(format!("ODBC: cannot decode BigDecimal: {:?}", value).into())
     }
 }

sqlx-core/src/odbc/types/bytes.rs

@@ -194,13 +194,7 @@ mod tests {
             int: None,
             float: None,
         };
-
-        let result = <Vec<u8> as Decode<Odbc>>::decode(value);
-        assert!(result.is_err());
-        assert_eq!(
-            result.unwrap_err().to_string(),
-            "ODBC: cannot decode Vec<u8>"
-        );
+        assert!(<Vec<u8> as Decode<'_, Odbc>>::decode(value).is_err());
     }
 
     #[test]