Skip to content

Commit 0a43560

Browse files
committed
fix(odbc): improve error handling in column collection and statement preparation
This commit updates the `collect_columns` and `create_column` functions to return `Result` types, enhancing error handling during column collection. Additionally, it refines tests for prepared statements to ensure proper error assertions when executing invalid SQL queries and binding incorrect parameter types.
1 parent 6d3ed6b commit 0a43560

File tree

2 files changed

+36
-49
lines changed

2 files changed

+36
-49
lines changed

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

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,24 @@ mod executor;
2424
type PreparedStatement = Prepared<StatementConnection<SharedConnection<'static>>>;
2525
type SharedPreparedStatement = Arc<Mutex<PreparedStatement>>;
2626

27-
fn collect_columns(prepared: &mut PreparedStatement) -> Vec<OdbcColumn> {
28-
let count = prepared.num_result_cols().unwrap_or(0);
29-
(1..=count)
30-
.map(|i| create_column(prepared, i as u16))
31-
.collect()
27+
fn collect_columns(prepared: &mut PreparedStatement) -> Result<Vec<OdbcColumn>, Error> {
28+
let count = prepared.num_result_cols()?;
29+
let mut columns = Vec::with_capacity(count as usize);
30+
for i in 1..=count {
31+
columns.push(create_column(prepared, i as u16)?);
32+
}
33+
Ok(columns)
3234
}
3335

34-
fn create_column(stmt: &mut PreparedStatement, index: u16) -> OdbcColumn {
36+
fn create_column(stmt: &mut PreparedStatement, index: u16) -> Result<OdbcColumn, Error> {
3537
let mut cd = odbc_api::ColumnDescription::default();
36-
let _ = stmt.describe_col(index, &mut cd);
38+
stmt.describe_col(index, &mut cd)?;
3739

38-
OdbcColumn {
40+
Ok(OdbcColumn {
3941
name: decode_column_name(cd.name, index),
4042
type_info: OdbcTypeInfo::new(cd.data_type),
4143
ordinal: usize::from(index.checked_sub(1).unwrap()),
42-
}
44+
})
4345
}
4446

4547
pub(super) trait ColumnNameDecode {
@@ -191,8 +193,8 @@ impl OdbcConnection {
191193
let (prepared, metadata) = spawn_blocking(move || {
192194
let mut prepared = conn.into_prepared(&sql_clone)?;
193195
let metadata = OdbcStatementMetadata {
194-
columns: collect_columns(&mut prepared),
195-
parameters: usize::from(prepared.num_params().unwrap_or(0)),
196+
columns: collect_columns(&mut prepared)?,
197+
parameters: usize::from(prepared.num_params()?),
196198
};
197199
Ok::<_, Error>((prepared, metadata))
198200
})

tests/odbc/odbc.rs

Lines changed: 23 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -947,34 +947,17 @@ async fn it_handles_prepare_statement_errors() -> anyhow::Result<()> {
947947
}
948948

949949
// Test executing prepared SQL with syntax errors
950-
match conn
950+
let res = conn
951951
.prepare("SELECT idonotexist FROM idonotexist WHERE idonotexist")
952-
.await
953-
{
954-
Ok(stmt) => match stmt.query().fetch_one(&mut conn).await {
955-
Ok(_) => panic!("should be an error"),
956-
Err(sqlx_oldapi::Error::Database(err)) => {
957-
assert!(
958-
err.to_string().contains("idonotexist"),
959-
"{:?} should contain 'idonotexist'",
960-
err
961-
);
962-
}
963-
Err(err) => {
964-
panic!("should be a database error, got {:?}", err);
965-
}
966-
},
967-
Err(sqlx_oldapi::Error::Database(err)) => {
968-
assert!(
969-
err.to_string().to_lowercase().contains("idonotexist"),
970-
"{:?} should contain 'idonotexist'",
971-
err
972-
);
973-
}
974-
Err(err) => {
975-
panic!("should be an error, got {:?}", err);
976-
}
977-
}
952+
.await;
953+
let Err(sqlx_oldapi::Error::Database(err)) = res else {
954+
panic!("should be an error, got {:?}", res);
955+
};
956+
assert!(
957+
err.to_string().to_lowercase().contains("idonotexist"),
958+
"{:?} should contain 'idonotexist'",
959+
err
960+
);
978961
Ok(())
979962
}
980963

@@ -1185,23 +1168,25 @@ async fn it_handles_concurrent_error_scenarios() -> anyhow::Result<()> {
11851168
}
11861169

11871170
#[tokio::test]
1188-
async fn it_handles_prepared_statement_with_wrong_parameters() -> anyhow::Result<()> {
1171+
async fn it_handles_prepared_statement_with_wrong_parameter_count() -> anyhow::Result<()> {
11891172
let mut conn = new::<Odbc>().await?;
11901173

11911174
// Prepare a statement expecting specific parameter types
1192-
let stmt = conn.prepare("SELECT ? + ? AS sum").await?;
1175+
let stmt = conn.prepare("SELECT ? AS a, ? as b").await?;
11931176

11941177
// Test binding incompatible types (if the database is strict about types)
11951178
// Some databases/drivers are permissive, others are strict
1196-
let result = stmt
1197-
.query()
1198-
.bind("not_a_number")
1199-
.bind("also_not_a_number")
1200-
.fetch_one(&mut conn)
1201-
.await;
1202-
// This may or may not error depending on the database's type system
1203-
let _ = result;
1204-
1179+
let result = stmt.query().bind(42_i32).fetch_one(&mut conn).await;
1180+
let Err(sqlx_oldapi::Error::Database(err)) = result else {
1181+
panic!("should be an error, got {:?}", result);
1182+
};
1183+
// https://learn.microsoft.com/en-us/sql/odbc/reference/appendixes/appendix-a-odbc-error-codes?view=sql-server-ver17
1184+
// 07002 -> COUNT field incorrect
1185+
assert!(
1186+
err.to_string().contains("07002"),
1187+
"{:?} should contain '07002' (COUNT field incorrect)",
1188+
err
1189+
);
12051190
Ok(())
12061191
}
12071192

0 commit comments

Comments
 (0)