refactor(odbc): introduce OdbcBatch for reduced cloning

lovasoa · lovasoa · commit e9eb8d05aaba · 2025-09-29T11:51:46.000+02:00
This commit refactors the OdbcRow structure to utilize an OdbcBatch, allowing for better management of column data and row indices. The OdbcBatch encapsulates the columns and their associated data, streamlining the retrieval process and enhancing the overall architecture of the ODBC module.
diff --git a/sqlx-core/src/odbc/connection/odbc_bridge.rs b/sqlx-core/src/odbc/connection/odbc_bridge.rs
@@ -1,8 +1,8 @@
 use super::decode_column_name;
 use crate::error::Error;
 use crate::odbc::{
-    connection::MaybePrepared, ColumnData, OdbcArgumentValue, OdbcArguments, OdbcColumn,
-    OdbcQueryResult, OdbcRow, OdbcTypeInfo, OdbcValue,
+    connection::MaybePrepared, ColumnData, OdbcArgumentValue, OdbcArguments, OdbcBatch, OdbcColumn,
+    OdbcQueryResult, OdbcRow, OdbcTypeInfo,
 };
 use either::Either;
 use flume::{SendError, Sender};
@@ -247,26 +247,24 @@ where
     let mut receiver_open = true;
 
     while let Some(batch) = row_set_cursor.fetch()? {
-        let columns: Vec<_> = bindings.iter().map(|b| b.column.clone()).collect();
-
         // Create ColumnData instances that can be shared across rows
-        let column_data_vec: Vec<_> = bindings
+        let column_data: Vec<_> = bindings
             .iter()
             .enumerate()
             .map(|(col_index, binding)| {
-                create_column_data(batch.column(col_index), &binding.column.type_info)
+                create_column_data(batch.column(col_index), &binding.column)
             })
             .collect();
 
-        for row_index in 0..batch.num_rows() {
-            let row_values: Vec<_> = column_data_vec
-                .iter()
-                .map(|column_data| OdbcValue::new(Arc::clone(column_data), row_index))
-                .collect();
+        let odbc_batch = Arc::new(OdbcBatch {
+            columns: bindings.iter().map(|b| b.column.clone()).collect(),
+            column_data,
+        });
 
+        for row_index in 0..batch.num_rows() {
             let row = OdbcRow {
-                columns: columns.clone(),
-                values: row_values,
+                row_index,
+                batch: Arc::clone(&odbc_batch),
             };
 
             if send_row(tx, row).is_err() {
@@ -283,13 +281,13 @@ where
     Ok(receiver_open)
 }
 
-fn create_column_data(slice: AnySlice<'_>, type_info: &OdbcTypeInfo) -> Arc<ColumnData> {
+fn create_column_data(slice: AnySlice<'_>, column: &OdbcColumn) -> Arc<ColumnData> {
     use crate::odbc::value::convert_any_slice_to_value_vec;
 
     let (values, nulls) = convert_any_slice_to_value_vec(slice);
     Arc::new(ColumnData {
         values,
-        type_info: type_info.clone(),
+        type_info: column.type_info.clone(),
         nulls,
     })
 }
diff --git a/sqlx-core/src/odbc/mod.rs b/sqlx-core/src/odbc/mod.rs
@@ -43,7 +43,7 @@ pub use connection::OdbcConnection;
 pub use database::Odbc;
 pub use options::OdbcConnectOptions;
 pub use query_result::OdbcQueryResult;
-pub use row::OdbcRow;
+pub use row::{OdbcBatch, OdbcRow};
 pub use statement::{OdbcStatement, OdbcStatementMetadata};
 pub use transaction::OdbcTransactionManager;
 pub use type_info::{DataTypeExt, OdbcTypeInfo};
diff --git a/sqlx-core/src/odbc/row.rs b/sqlx-core/src/odbc/row.rs
@@ -1,21 +1,27 @@
 use crate::column::ColumnIndex;
 use crate::database::HasValueRef;
 use crate::error::Error;
-use crate::odbc::{Odbc, OdbcColumn, OdbcValue};
+use crate::odbc::{Odbc, OdbcColumn, OdbcValueRef};
 use crate::row::Row;
-use crate::value::Value;
+use std::sync::Arc;
 
 #[derive(Debug, Clone)]
-pub struct OdbcRow {
+pub struct OdbcBatch {
     pub(crate) columns: Vec<OdbcColumn>,
-    pub(crate) values: Vec<OdbcValue>,
+    pub(crate) column_data: Vec<Arc<crate::odbc::ColumnData>>,
+}
+
+#[derive(Debug, Clone)]
+pub struct OdbcRow {
+    pub(crate) row_index: usize,
+    pub(crate) batch: Arc<OdbcBatch>,
 }
 
 impl Row for OdbcRow {
     type Database = Odbc;
 
     fn columns(&self) -> &[OdbcColumn] {
-        &self.columns
+        &self.batch.columns
     }
 
     fn try_get_raw<I>(
@@ -25,21 +31,21 @@ impl Row for OdbcRow {
     where
         I: ColumnIndex<Self>,
     {
-        let idx = index.index(self)?;
-        let value = &self.values[idx];
-        Ok(value.as_ref())
+        let column_index = index.index(self)?;
+        Ok(OdbcValueRef::new(&self.batch, self.row_index, column_index))
     }
 }
 
 impl ColumnIndex<OdbcRow> for &str {
     fn index(&self, row: &OdbcRow) -> Result<usize, Error> {
         // Try exact match first (for performance)
-        if let Some(pos) = row.columns.iter().position(|col| col.name == *self) {
+        if let Some(pos) = row.batch.columns.iter().position(|col| col.name == *self) {
             return Ok(pos);
         }
 
         // Fall back to case-insensitive match (for databases like Snowflake)
-        row.columns
+        row.batch
+            .columns
             .iter()
             .position(|col| col.name.eq_ignore_ascii_case(self))
             .ok_or_else(|| Error::ColumnNotFound((*self).into()))
@@ -56,6 +62,7 @@ mod private {
 impl From<OdbcRow> for crate::any::AnyRow {
     fn from(row: OdbcRow) -> Self {
         let columns = row
+            .batch
             .columns
             .iter()
             .map(|col| crate::any::AnyColumn {
@@ -81,58 +88,48 @@ mod tests {
     use std::sync::Arc;
 
     fn create_test_row() -> OdbcRow {
-        use crate::odbc::OdbcValue;
+        let columns = vec![
+            OdbcColumn {
+                name: "lowercase_col".to_string(),
+                type_info: OdbcTypeInfo::new(DataType::Integer),
+                ordinal: 0,
+            },
+            OdbcColumn {
+                name: "UPPERCASE_COL".to_string(),
+                type_info: OdbcTypeInfo::new(DataType::Varchar { length: None }),
+                ordinal: 1,
+            },
+            OdbcColumn {
+                name: "MixedCase_Col".to_string(),
+                type_info: OdbcTypeInfo::new(DataType::Double),
+                ordinal: 2,
+            },
+        ];
+
+        let column_data = vec![
+            ColumnData {
+                values: OdbcValueVec::NullableBigInt(vec![Some(42)]),
+                type_info: OdbcTypeInfo::new(DataType::Integer),
+                nulls: vec![false],
+            },
+            ColumnData {
+                values: OdbcValueVec::Text(vec![Some("test".to_string())]),
+                type_info: OdbcTypeInfo::new(DataType::Varchar { length: None }),
+                nulls: vec![false],
+            },
+            ColumnData {
+                values: OdbcValueVec::NullableDouble(vec![Some(std::f64::consts::PI)]),
+                type_info: OdbcTypeInfo::new(DataType::Double),
+                nulls: vec![false],
+            },
+        ];
 
         OdbcRow {
-            columns: vec![
-                OdbcColumn {
-                    name: "lowercase_col".to_string(),
-                    type_info: OdbcTypeInfo::new(DataType::Integer),
-                    ordinal: 0,
-                },
-                OdbcColumn {
-                    name: "UPPERCASE_COL".to_string(),
-                    type_info: OdbcTypeInfo::new(DataType::Varchar { length: None }),
-                    ordinal: 1,
-                },
-                OdbcColumn {
-                    name: "MixedCase_Col".to_string(),
-                    type_info: OdbcTypeInfo::new(DataType::Double),
-                    ordinal: 2,
-                },
-            ],
-            values: vec![
-                {
-                    let column = ColumnData {
-                        values: OdbcValueVec::NullableBigInt(vec![Some(42)]),
-                        type_info: OdbcTypeInfo::new(DataType::Integer),
-                    };
-                    OdbcValue {
-                        column_data: Arc::new(column),
-                        row_index: 0,
-                    }
-                },
-                {
-                    let column = ColumnData {
-                        values: OdbcValueVec::Text(vec![Some("test".to_string())]),
-                        type_info: OdbcTypeInfo::new(DataType::Varchar { length: None }),
-                    };
-                    OdbcValue {
-                        column_data: Arc::new(column),
-                        row_index: 0,
-                    }
-                },
-                {
-                    let column = ColumnData {
-                        values: OdbcValueVec::NullableDouble(vec![Some(std::f64::consts::PI)]),
-                        type_info: OdbcTypeInfo::new(DataType::Double),
-                    };
-                    OdbcValue {
-                        column_data: Arc::new(column),
-                        row_index: 0,
-                    }
-                },
-            ],
+            row_index: 0,
+            batch: Arc::new(OdbcBatch {
+                columns,
+                column_data,
+            }),
         }
     }
 
diff --git a/sqlx-core/src/odbc/types/chrono.rs b/sqlx-core/src/odbc/types/chrono.rs
@@ -254,7 +254,7 @@ impl<'r> Decode<'r, Odbc> for NaiveDate {
 
         Err(format!(
             "ODBC: cannot decode NaiveDate from value with type '{}'",
-            value.column_data.type_info.name()
+            value.batch.columns[value.column_index].type_info.name()
         )
         .into())
     }
diff --git a/sqlx-core/src/odbc/value.rs b/sqlx-core/src/odbc/value.rs

Original file line number	Diff line number	Diff line change
`@@ -254,7 +254,7 @@ impl<'r> Decode<'r, Odbc> for NaiveDate {`
`254`	`254`
`255`	`255`	`Err(format!(`
`256`	`256`	`"ODBC: cannot decode NaiveDate from value with type '{}'",`
`257`		`- value.column_data.type_info.name()`
	`257`	`+ value.batch.columns[value.column_index].type_info.name()`
`258`	`258`	`)`
`259`	`259`	`.into())`
`260`	`260`	`}`