refactor(odbc): improve error handling and data fetching logic

This commit enhances the error handling in OdbcValueRef by utilizing the new MismatchedTypeError for better type mismatch reporting. It also simplifies the data fetching logic in OdbcBridge by consolidating the handling of default values and null checks, improving code clarity and maintainability. Additionally, tests are updated to reflect these changes, ensuring robust functionality across buffered and unbuffered modes.

Author: lovasoa

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

@@ -434,9 +434,7 @@ where
         vec: &mut Vec<T>,
         nulls: &mut Vec<bool>,
     ) {
-        let mut tmp = T::default();
-        nulls.push(cursor_row.get_data(col_index, &mut tmp).is_ok());
-        vec.push(tmp);
+        push_get_data_with_default(cursor_row, col_index, vec, nulls, T::default());
     }
 
     fn push_get_data_with_default<T: Copy + CElement + CDataMut>(
@@ -447,13 +445,8 @@ where
         default_val: T,
     ) {
         let mut tmp = default_val;
-        if cursor_row.get_data(col_index, &mut tmp).is_ok() {
-            vec.push(tmp);
-            nulls.push(false);
-        } else {
-            vec.push(default_val);
-            nulls.push(true);
-        }
+        nulls.push(cursor_row.get_data(col_index, &mut tmp).is_err());
+        vec.push(tmp);
     }
 
     fn push_binary(
@@ -463,16 +456,8 @@ where
         nulls: &mut Vec<bool>,
     ) {
         let mut buf = Vec::new();
-        match cursor_row.get_text(col_index, &mut buf) {
-            Ok(true) => {
-                vec.push(buf);
-                nulls.push(false);
-            }
-            Ok(false) | Err(_) => {
-                vec.push(Vec::new());
-                nulls.push(true);
-            }
-        }
+        nulls.push(cursor_row.get_text(col_index, &mut buf).is_err());
+        vec.push(buf);
     }
 
     fn push_text(

sqlx-core/src/odbc/value.rs

@@ -1,4 +1,5 @@
 use crate::odbc::{Odbc, OdbcBatch, OdbcTypeInfo};
+use crate::type_info::TypeInfo;
 use crate::value::{Value, ValueRef};
 use odbc_api::buffers::{AnySlice, NullableSlice};
 use odbc_api::sys::NULL_DATA;
@@ -156,32 +157,30 @@ impl<'r> OdbcValueRef<'r> {
     }
 
     pub fn try_int<T: TryFromInt + crate::types::Type<Odbc>>(&self) -> crate::error::Result<T> {
-        if !T::compatible(&self.batch.column_data[self.column_index].type_info) {
-            return Err(crate::error::Error::Decode(
-                crate::error::mismatched_types::<Odbc, T>(
-                    &self.batch.column_data[self.column_index].type_info,
-                ),
-            ));
-        }
         self.int::<T>().ok_or_else(|| {
-            crate::error::Error::Decode(crate::error::mismatched_types::<Odbc, T>(
-                &self.batch.columns[self.column_index].type_info,
-            ))
+            crate::error::Error::Decode(Box::new(crate::error::MismatchedTypeError {
+                rust_type: T::type_info().name().to_string(),
+                rust_sql_type: T::type_info().name().to_string(),
+                sql_type: self.batch.column_data[self.column_index]
+                    .type_info
+                    .name()
+                    .to_string(),
+                source: Some(format!("ODBC: cannot decode {:?}", self).into()),
+            }))
         })
     }
 
     pub fn try_float<T: TryFromFloat + crate::types::Type<Odbc>>(&self) -> crate::error::Result<T> {
-        if !T::compatible(&self.batch.column_data[self.column_index].type_info) {
-            return Err(crate::error::Error::Decode(
-                crate::error::mismatched_types::<Odbc, T>(
-                    &self.batch.column_data[self.column_index].type_info,
-                ),
-            ));
-        }
         self.float::<T>().ok_or_else(|| {
-            crate::error::Error::Decode(crate::error::mismatched_types::<Odbc, T>(
-                &self.batch.columns[self.column_index].type_info,
-            ))
+            crate::error::Error::Decode(Box::new(crate::error::MismatchedTypeError {
+                rust_type: T::type_info().name().to_string(),
+                rust_sql_type: T::type_info().name().to_string(),
+                sql_type: self.batch.column_data[self.column_index]
+                    .type_info
+                    .name()
+                    .to_string(),
+                source: Some(format!("ODBC: cannot decode {:?}", self).into()),
+            }))
         })
     }
 

tests/odbc/odbc.rs

@@ -1037,25 +1037,45 @@ async fn it_handles_prepared_statement_with_wrong_parameters() -> anyhow::Result
 }
 
 #[tokio::test]
-async fn it_works_with_unbuffered_mode() -> anyhow::Result<()> {
+async fn it_works_with_buffered_and_unbuffered_mode() -> anyhow::Result<()> {
     use sqlx_oldapi::odbc::{OdbcBufferSettings, OdbcConnectOptions};
 
     // Create connection with unbuffered settings
     let database_url = std::env::var("DATABASE_URL").unwrap();
     let mut opts = OdbcConnectOptions::from_str(&database_url)?;
 
-    for batch_size in [1, 100, 10000] {
-        opts.buffer_settings(OdbcBufferSettings {
-            batch_size,
+    let count = 450;
+
+    let select = (0..count)
+        .map(|i| format!("SELECT {i} AS n, '{}' as aas", "a".repeat(i)))
+        .collect::<Vec<_>>()
+        .join(" UNION ALL ");
+
+    for buf_settings in [
+        OdbcBufferSettings {
+            batch_size: 1,
+            max_column_size: None,
+        },
+        OdbcBufferSettings {
+            batch_size: 1,
+            max_column_size: Some(450),
+        },
+        OdbcBufferSettings {
+            batch_size: 100,
             max_column_size: None,
-        });
+        },
+        OdbcBufferSettings {
+            batch_size: 10000,
+            max_column_size: None,
+        },
+        OdbcBufferSettings {
+            batch_size: 10000,
+            max_column_size: Some(450),
+        },
+    ] {
+        opts.buffer_settings(buf_settings);
 
         let mut conn = OdbcConnection::connect_with(&opts).await?;
-        let count = 450;
-        let select = (0..count)
-            .map(|i| format!("SELECT {i} AS n"))
-            .collect::<Vec<_>>()
-            .join(" UNION ALL ");
 
         // Test that unbuffered mode works correctly
         let s = conn
@@ -1067,8 +1087,20 @@ async fn it_works_with_unbuffered_mode() -> anyhow::Result<()> {
         assert_eq!(s.len(), count);
         for i in 0..count {
             let row = s.get(i).expect("row expected");
-            let as_i64 = row.get::<'_, i64, _>(0);
+            let as_i64 = row
+                .try_get_raw(0)
+                .expect("1 column expected")
+                .to_owned()
+                .try_decode::<i64>()
+                .expect("SELECT n should be an i64");
             assert_eq!(as_i64, i64::try_from(i).unwrap());
+            let aas = row
+                .try_get_raw(1)
+                .expect("1 column expected")
+                .to_owned()
+                .try_decode::<String>()
+                .expect("SELECT aas should be a string");
+            assert_eq!(aas, "a".repeat(i));
         }
     }
     Ok(())