From 35c6b57476165f16ff39a28c615f8e549aaf5ff8 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 13 Feb 2026 13:38:58 +0100 Subject: [PATCH 1/7] wip --- src/lib.rs | 4 ---- src/odbc/api/sqlconnect.rs | 2 +- src/odbc/api/sqldriverconnect.rs | 8 +++++--- src/odbc/api/sqlgetfunctions.rs | 1 + src/odbc/api/sqlgetinfo.rs | 2 +- src/odbc/def/c_data_type.rs | 5 +++-- src/odbc/implementation/alloc_handles.rs | 5 ++++- tests/basic_odbc_connection_test.rs | 4 +++- 8 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 918953d..d6971c6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,25 +10,21 @@ mod odbc; // Windows: Use the built-in Windows ODBC library #[cfg_attr(windows, link(name = "odbc32"))] - // Unix + Dynamic + unixODBC (default case for Linux) #[cfg_attr( all(not(windows), not(feature = "static"), not(feature = "iodbc")), link(name = "odbcinst") )] - // Unix + Static + unixODBC (for self-contained binaries) #[cfg_attr( all(not(windows), feature = "static", not(feature = "iodbc")), link(name = "odbcinst", kind = "static") )] - // Unix + Dynamic + iODBC (common on macOS) #[cfg_attr( all(not(windows), not(feature = "static"), feature = "iodbc"), link(name = "iodbcinst") )] - // Unix + Static + iODBC (self-contained binaries with iODBC) #[cfg_attr( all(not(windows), feature = "static", feature = "iodbc"), diff --git a/src/odbc/api/sqlconnect.rs b/src/odbc/api/sqlconnect.rs index e6f6108..a0ccea9 100644 --- a/src/odbc/api/sqlconnect.rs +++ b/src/odbc/api/sqlconnect.rs @@ -1,5 +1,5 @@ //! -//! https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqlconnect-function?view=sql-server-ver16 +//! //! //! ```c //! SQLRETURN SQLConnect( diff --git a/src/odbc/api/sqldriverconnect.rs b/src/odbc/api/sqldriverconnect.rs index 90591bd..252f448 100644 --- a/src/odbc/api/sqldriverconnect.rs +++ b/src/odbc/api/sqldriverconnect.rs @@ -1,5 +1,5 @@ //! -//! https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqldriverconnect-function +//! //! //! ```c //! SQLRETURN SQLDriverConnect( @@ -18,7 +18,7 @@ use crate::odbc::implementation::connect::impl_connect; use crate::odbc::utils::{get_from_wrapper, maybe_utf16_to_string}; use odbc_sys::{HandleType, SmallInt, SqlReturn, USmallInt, WChar}; use std::ffi::c_void; -use tracing::{debug, error, info}; +use tracing::{error, info}; /// SQLDriverConnect establishes connections to a driver and a data source using a connection string /// @@ -64,7 +64,8 @@ pub extern "C" fn SQLDriverConnectW( } }; - debug!("Connection string: {}", connection_string); + println!("Connection string: {}", connection_string); + //println!("Connection string parts: {:#?}", connection_string.split(';').collect::>()); // TODO: Parse connection string properly (DSN=..., Database=..., etc.) // For now, just extract database path from a simple connection string @@ -81,6 +82,7 @@ pub extern "C" fn SQLDriverConnectW( // No database specified - this is an error None }; + println!("DEBUG: database_path result: {:?}", database_path); match database_path { Some(db_path) => { diff --git a/src/odbc/api/sqlgetfunctions.rs b/src/odbc/api/sqlgetfunctions.rs index 729cb11..c2b8ff3 100644 --- a/src/odbc/api/sqlgetfunctions.rs +++ b/src/odbc/api/sqlgetfunctions.rs @@ -1,3 +1,4 @@ +#![allow(dead_code)] use odbc_sys::{SqlReturn, USmallInt}; use std::ffi::c_void; use std::slice; diff --git a/src/odbc/api/sqlgetinfo.rs b/src/odbc/api/sqlgetinfo.rs index 63a90b0..d4acb49 100644 --- a/src/odbc/api/sqlgetinfo.rs +++ b/src/odbc/api/sqlgetinfo.rs @@ -9,7 +9,7 @@ const STRING_LENGTH_FOR_UINTEGER: i16 = std::mem::size_of::() as i16; /// Returns general information about the driver and data source associated with a connection /// -/// https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgetinfo-function +/// /// /// # Returns /// `SUCCESS`, `SUCCESS_WITH_INFO`, `ERROR`, or `INVALID_HANDLE` diff --git a/src/odbc/def/c_data_type.rs b/src/odbc/def/c_data_type.rs index 6fe1b2e..0ac754a 100644 --- a/src/odbc/def/c_data_type.rs +++ b/src/odbc/def/c_data_type.rs @@ -1,3 +1,4 @@ +#![allow(dead_code)] /// Extended C Types range 4000 and above. Range of -100 thru 200 is reserved by Driver Manager. /// `SQL_C_TYPES_EXTENDED`. pub const C_TYPES_EXTENDED: i16 = 0x04000; @@ -82,5 +83,5 @@ pub enum CDataType { #[cfg(windows)] pub use CDataType::ULong as UBigInt; -#[cfg(not(windows))] -pub use CDataType::ULong as Bookmark; +//#[cfg(not(windows))] +//pub use CDataType::ULong as Bookmark; diff --git a/src/odbc/implementation/alloc_handles.rs b/src/odbc/implementation/alloc_handles.rs index 5d850d7..0120532 100644 --- a/src/odbc/implementation/alloc_handles.rs +++ b/src/odbc/implementation/alloc_handles.rs @@ -1,3 +1,4 @@ +#![allow(dead_code)] use odbc_sys::AttrOdbcVersion; use rusqlite::{Connection, Row, Rows, Statement}; @@ -38,7 +39,9 @@ pub(crate) fn impl_allocate_dbc_handle(_env_handle: &mut EnvironmentHandle) -> C } } -pub(crate) fn allocate_stmt_handle(connection_handle: &mut ConnectionHandle) -> StatementHandle<'_> { +pub(crate) fn allocate_stmt_handle( + connection_handle: &mut ConnectionHandle, +) -> StatementHandle<'_> { let connection_ref = connection_handle.sqlite_connection.as_ref().unwrap(); StatementHandle { sqlite_connection: connection_ref, diff --git a/tests/basic_odbc_connection_test.rs b/tests/basic_odbc_connection_test.rs index 33a500e..2da577b 100644 --- a/tests/basic_odbc_connection_test.rs +++ b/tests/basic_odbc_connection_test.rs @@ -1,5 +1,6 @@ +#![allow(dead_code)] use odbc_api::buffers::TextRowSet; -use odbc_api::{ConnectionOptions, Cursor, Environment, ResultSetMetadata}; +use odbc_api::{ConnectionOptions, Cursor, Environment}; use std::process::Command; /// Basic ODBC connection test using odbc-api crate @@ -8,6 +9,7 @@ use std::process::Command; /// to our driver through the standard ODBC client stack. const CONNECTION_STRING: &str = "DSN=test_connection"; +//const CONNECTION_STRING: &str = "Driver=/home/andrew/gitrepos/odbc-rs-sqlite/target/debug/libodbc_driver_rs.so;Database=/home/andrew/gitrepos/odbc-rs-sqlite/test_odbc.sqlite"; /// Set up test environment by building driver and configuring ODBC fn setup_test_environment() -> std::result::Result<(), Box> { From 12c4d3ab4d6739bbd95aeb558d7b6c401156c10e Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 13 Feb 2026 14:24:40 +0100 Subject: [PATCH 2/7] Fix clippy warnings and add DSN resolution to SQLDriverConnectW - Fix all 16 clippy warnings: empty line after attr, let-and-return, needless borrows, uninlined format args, clone-on-copy, module inception, enum variant names, too-many-arguments - Add DSN resolution to SQLDriverConnectW so connection strings with DSN= (but no Database=) resolve the database path from odbc.ini - Extract impl_connect_to_database for direct database path connections, avoiding double DSN lookup when called from SQLDriverConnectW - Untrack .claude/settings.local.json (already in .gitignore) Co-Authored-By: Claude Opus 4.6 --- .claude/settings.local.json | 9 ------ src/lib.rs | 1 - src/odbc/api/sqlallochandle.rs | 6 ++-- src/odbc/api/sqlcolattribute.rs | 4 +-- src/odbc/api/sqldriverconnect.rs | 41 ++++++++++++++++++++++++---- src/odbc/api/sqlspecialcolumns.rs | 2 +- src/odbc/api/sqlstatistics.rs | 2 +- src/odbc/implementation.rs | 1 + src/odbc/implementation/connect.rs | 13 ++++++++- src/odbc/implementation/env_attrs.rs | 2 +- src/odbc/implementation/getdata.rs | 4 +-- src/odbc/utils.rs | 5 ++-- 12 files changed, 60 insertions(+), 30 deletions(-) delete mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json deleted file mode 100644 index c36ca14..0000000 --- a/.claude/settings.local.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "permissions": { - "allow": [ - "Bash(cargo test:*)", - "Bash(./scripts/setup-test-db.sh:*)" - ], - "deny": [] - } -} \ No newline at end of file diff --git a/src/lib.rs b/src/lib.rs index d6971c6..0c0d785 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,7 +30,6 @@ mod odbc; all(not(windows), feature = "static", feature = "iodbc"), link(name = "iodbcinst", kind = "static") )] - // This function is here because it's needed for ODBC configuration file parsing // but doesn't exist in the standard odbc-sys crate. // It was added here directly temporarily, eventually this might move to a separate file or library. diff --git a/src/odbc/api/sqlallochandle.rs b/src/odbc/api/sqlallochandle.rs index a1534a7..66b96c7 100644 --- a/src/odbc/api/sqlallochandle.rs +++ b/src/odbc/api/sqlallochandle.rs @@ -42,7 +42,7 @@ pub extern "C" fn SQLAllocHandle( // From the spec: // When allocating a handle other than an environment handle, if SQLAllocHandle returns SQL_ERROR, it sets OutputHandlePtr to SQL_NULL_HDBC, SQL_NULL_HSTMT, or SQL_NULL_HDESC, depending on the value of HandleType, unless the output argument is a null pointer. // The application can then obtain additional information from the diagnostic data structure associated with the handle in the InputHandle argument. - let result = match handle_type { + match handle_type { HandleType::Env => { // Spec: If HandleType is SQL_HANDLE_ENV, this is SQL_NULL_HANDLE. if !input_handle.is_null() { @@ -106,9 +106,7 @@ pub extern "C" fn SQLAllocHandle( } HandleType::Desc => SqlReturn::SUCCESS, HandleType::DbcInfoToken => SqlReturn::SUCCESS, - }; - - result + } } #[cfg(test)] diff --git a/src/odbc/api/sqlcolattribute.rs b/src/odbc/api/sqlcolattribute.rs index 43c14dd..cd332fd 100644 --- a/src/odbc/api/sqlcolattribute.rs +++ b/src/odbc/api/sqlcolattribute.rs @@ -103,7 +103,7 @@ pub extern "C" fn SQLColAttributeW( // Return column name match stmt.column_name(col_index) { Ok(name) => return_string_attribute( - &name, + name, character_attribute_ptr, buffer_length, string_length_ptr, @@ -172,7 +172,7 @@ pub extern "C" fn SQLColAttributeW( // Return column label (same as name for SQLite) match stmt.column_name(col_index) { Ok(name) => return_string_attribute( - &name, + name, character_attribute_ptr, buffer_length, string_length_ptr, diff --git a/src/odbc/api/sqldriverconnect.rs b/src/odbc/api/sqldriverconnect.rs index 252f448..276ee1a 100644 --- a/src/odbc/api/sqldriverconnect.rs +++ b/src/odbc/api/sqldriverconnect.rs @@ -14,8 +14,8 @@ //! ``` use crate::odbc::implementation::alloc_handles::ConnectionHandle; -use crate::odbc::implementation::connect::impl_connect; -use crate::odbc::utils::{get_from_wrapper, maybe_utf16_to_string}; +use crate::odbc::implementation::connect::impl_connect_to_database; +use crate::odbc::utils::{get_from_wrapper, get_private_profile_string, maybe_utf16_to_string}; use odbc_sys::{HandleType, SmallInt, SqlReturn, USmallInt, WChar}; use std::ffi::c_void; use tracing::{error, info}; @@ -64,7 +64,7 @@ pub extern "C" fn SQLDriverConnectW( } }; - println!("Connection string: {}", connection_string); + println!("Connection string: {connection_string}"); //println!("Connection string parts: {:#?}", connection_string.split(';').collect::>()); // TODO: Parse connection string properly (DSN=..., Database=..., etc.) @@ -78,18 +78,47 @@ pub extern "C" fn SQLDriverConnectW( .find(|part| part.starts_with("Database=")) .and_then(|part| part.strip_prefix("Database=")) .map(|path| path.trim().to_string()) + } else if connection_string.contains("DSN=") { + // Resolve Database from DSN configuration + let dsn = connection_string + .split(';') + .find(|part| part.starts_with("DSN=")) + .and_then(|part| part.strip_prefix("DSN=")) + .map(|s| s.trim().to_string()); + + if let Some(dsn_name) = dsn { + match get_private_profile_string(&dsn_name, "Database", "odbc.ini", 1024) { + Ok(Some(db)) => { + info!("Resolved Database={} from DSN={}", db, dsn_name); + Some(db) + } + Ok(None) => { + error!( + "DSN '{}' found but no Database setting configured", + dsn_name + ); + None + } + Err(e) => { + error!("Failed to look up DSN '{}': {}", dsn_name, e); + None + } + } + } else { + None + } } else { // No database specified - this is an error None }; - println!("DEBUG: database_path result: {:?}", database_path); + println!("DEBUG: database_path result: {database_path:?}"); match database_path { Some(db_path) => { info!("Connecting to database: {}", db_path); - // Use existing connection logic - impl_connect(connection_handle, db_path, None, None); + // Open the database directly — DSN resolution already happened above + impl_connect_to_database(connection_handle, db_path); // TODO: Copy connection string to output buffer if provided if !out_connection_string.is_null() && buffer_length > 0 { diff --git a/src/odbc/api/sqlspecialcolumns.rs b/src/odbc/api/sqlspecialcolumns.rs index 41a2225..bec3ca4 100644 --- a/src/odbc/api/sqlspecialcolumns.rs +++ b/src/odbc/api/sqlspecialcolumns.rs @@ -2,7 +2,7 @@ use odbc_sys::SqlReturn; use std::ffi::c_void; use tracing::info; -#[allow(non_snake_case)] +#[allow(non_snake_case, clippy::too_many_arguments)] #[unsafe(no_mangle)] pub fn SQLSpecialColumnsW( _statement_handle: *mut c_void, diff --git a/src/odbc/api/sqlstatistics.rs b/src/odbc/api/sqlstatistics.rs index 9ca9d61..4f9c249 100644 --- a/src/odbc/api/sqlstatistics.rs +++ b/src/odbc/api/sqlstatistics.rs @@ -2,7 +2,7 @@ use odbc_sys::SqlReturn; use std::ffi::c_void; use tracing::info; -#[allow(non_snake_case)] +#[allow(non_snake_case, clippy::too_many_arguments)] #[unsafe(no_mangle)] pub fn SQLStatisticsW( _statement_handle: *mut c_void, diff --git a/src/odbc/implementation.rs b/src/odbc/implementation.rs index 97bdad9..c9d14b1 100644 --- a/src/odbc/implementation.rs +++ b/src/odbc/implementation.rs @@ -2,4 +2,5 @@ pub(crate) mod alloc_handles; pub(crate) mod connect; pub(crate) mod env_attrs; pub(crate) mod getdata; +#[allow(clippy::module_inception)] pub(crate) mod implementation; diff --git a/src/odbc/implementation/connect.rs b/src/odbc/implementation/connect.rs index 2d8f6d7..594ee30 100644 --- a/src/odbc/implementation/connect.rs +++ b/src/odbc/implementation/connect.rs @@ -3,6 +3,8 @@ use crate::odbc::utils::get_private_profile_string; use rusqlite::{Connection, OpenFlags}; use tracing::{error, info}; +/// Connect via DSN name — looks up Database from ODBC configuration. +/// Used by SQLConnectW. pub(crate) fn impl_connect( connection_handle: &mut ConnectionHandle, server_name: String, @@ -21,8 +23,17 @@ pub(crate) fn impl_connect( } }; info!("Opening [{}] for DSN [{}]", database, server_name); + impl_connect_to_database(connection_handle, database); +} + +/// Connect directly to a database file path. +/// Used by SQLDriverConnectW after resolving the database path. +pub(crate) fn impl_connect_to_database( + connection_handle: &mut ConnectionHandle, + database_path: String, +) { let conn = match Connection::open_with_flags( - database, + &database_path, OpenFlags::SQLITE_OPEN_READ_WRITE | OpenFlags::SQLITE_OPEN_URI | OpenFlags::SQLITE_OPEN_NO_MUTEX, diff --git a/src/odbc/implementation/env_attrs.rs b/src/odbc/implementation/env_attrs.rs index 21336f7..52d34fc 100644 --- a/src/odbc/implementation/env_attrs.rs +++ b/src/odbc/implementation/env_attrs.rs @@ -7,5 +7,5 @@ pub(crate) fn set_odbc_version(env: &mut EnvironmentHandle, odbc_version: AttrOd } pub(crate) fn get_odbc_version(env: &EnvironmentHandle) -> AttrOdbcVersion { - env.odbc_version.clone() + env.odbc_version } diff --git a/src/odbc/implementation/getdata.rs b/src/odbc/implementation/getdata.rs index 520bfc8..26f8dce 100644 --- a/src/odbc/implementation/getdata.rs +++ b/src/odbc/implementation/getdata.rs @@ -45,7 +45,7 @@ pub(crate) fn impl_getdata( ValueRef::Blob(b) => { // Convert blob to hex string representation b.iter() - .map(|byte| format!("{:02x}", byte)) + .map(|byte| format!("{byte:02x}")) .collect::() } } @@ -101,7 +101,7 @@ pub(crate) fn impl_getdata( }, ValueRef::Blob(b) => b .iter() - .map(|byte| format!("{:02x}", byte)) + .map(|byte| format!("{byte:02x}")) .collect::(), } } diff --git a/src/odbc/utils.rs b/src/odbc/utils.rs index 8653131..101b39b 100644 --- a/src/odbc/utils.rs +++ b/src/odbc/utils.rs @@ -14,6 +14,7 @@ const DESC_TAG: i32 = 108498290; const DBC_INFO_TOKEN_TAG: i32 = 983280932; #[derive(Debug, Snafu)] +#[allow(clippy::enum_variant_names)] pub enum Error { #[snafu(display("SQLGetPrivateProfileStringW returned a negative value: {ret}"))] UnknownError { ret: i32 }, @@ -61,13 +62,13 @@ pub fn get_from_wrapper<'a, T>( ensure!( !wrapper_pointer.is_null(), NullPointerSnafu { - handle_type: handle_type.clone() + handle_type: *handle_type } ); let in_wrapper: &HandleWrapper = unsafe { &*(wrapper_pointer as *const HandleWrapper) }; - let tag = tag_for_handle(&handle_type); + let tag = tag_for_handle(handle_type); ensure!( in_wrapper.tag == tag, InvalidTagSnafu { From 0b0fef757e8f0b44524a2506fa95e5417dd05f27 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 13 Feb 2026 14:27:14 +0100 Subject: [PATCH 3/7] Add ODBC trace file to .gitignore Co-Authored-By: Claude Opus 4.6 --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 5a8f89a..7dde91a 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ test_odbc.sqlite lars_notes.md .claude/ +ODBC From c72d60518a3c044efea1634503d03642ac26e63a Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 13 Feb 2026 14:58:36 +0100 Subject: [PATCH 4/7] Remove unwraps from FFI code and implement SQLDescribeColW - Replace 3 .unwrap() calls in FFI functions with proper error handling that returns SqlReturn::ERROR instead of panicking the host application - Change allocate_stmt_handle to return Option so callers handle missing connections gracefully - Implement SQLDescribeColW with column name (UTF-16), type (SQL_VARCHAR), size (255), decimal digits (0), and nullability (SQL_NULLABLE) - Enable `wide` feature on odbc-api dev-dependency for proper Unicode ODBC client testing - Add test_describe_columns integration test Co-Authored-By: Claude Opus 4.6 --- Cargo.toml | 2 +- plan.md | 12 +- src/odbc/api/sqlallochandle.rs | 9 +- src/odbc/api/sqldescribecol.rs | 142 ++++++++++++++++++++--- src/odbc/api/sqltables.rs | 24 +++- src/odbc/implementation/alloc_handles.rs | 8 +- tests/basic_odbc_connection_test.rs | 50 +++++++- 7 files changed, 217 insertions(+), 30 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index aab9374..9f9b049 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,4 +48,4 @@ tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt"] } [dev-dependencies] -odbc-api = "14.3.0" +odbc-api = { version = "14.3.0", features = ["wide"] } diff --git a/plan.md b/plan.md index 2130960..914aa57 100644 --- a/plan.md +++ b/plan.md @@ -105,9 +105,10 @@ - [ ] **Fix StatementHandle lifetime issues** (`src/odbc/implementation/alloc_handles.rs:24-29`) - Replace raw references with proper ownership patterns - Ensure statements don't outlive connections -- [ ] **Remove all `.unwrap()` from FFI functions** - - Replace with proper error return codes - - Add error logging without panicking +- [DONE - 2026-02-13] **Remove all `.unwrap()` from FFI functions** + - SOLUTION: Replaced 3 `.unwrap()` calls in FFI code with proper error handling + - FILES: Modified src/odbc/api/sqltables.rs (prepare/query unwraps → match with SqlReturn::ERROR), src/odbc/implementation/alloc_handles.rs (allocate_stmt_handle returns Option), src/odbc/api/sqlallochandle.rs (handles None from allocate_stmt_handle) + - OUTCOME: No more panics possible from FFI code; all errors return SqlReturn::ERROR with logging ### **Phase 2: Robustness & Completeness** *Goal: Production-ready error handling and core functionality* @@ -127,6 +128,11 @@ - Implement proper handle deallocation based on handle type - Free both `HandleWrapper` and inner objects - Add comprehensive tests +- [DONE - 2026-02-13] **SQLDescribeColW implementation** + - SOLUTION: Full implementation returning column name (UTF-16), data type (SQL_VARCHAR), column size (255), decimal digits (0), nullable (SQL_NULLABLE) + - FILES: Rewrote src/odbc/api/sqldescribecol.rs, added test_describe_columns integration test + - ALSO: Enabled `wide` feature on odbc-api dev-dependency for proper Unicode ODBC client testing, discovered `cargo build` is needed before integration tests (cdylib loaded at runtime by DM) + - KEY INSIGHT: ODBC spec for SQLDescribeColW uses character counts (not byte counts) for buffer_length and name_length_ptr - [ ] **SQLDriverConnect implementation** - Parse connection strings properly - Support standard SQLite connection parameters diff --git a/src/odbc/api/sqlallochandle.rs b/src/odbc/api/sqlallochandle.rs index 66b96c7..0db5123 100644 --- a/src/odbc/api/sqlallochandle.rs +++ b/src/odbc/api/sqlallochandle.rs @@ -97,7 +97,14 @@ pub extern "C" fn SQLAllocHandle( } }; - let handle = allocate_stmt_handle(connection_handle); + let handle = match allocate_stmt_handle(connection_handle) { + Some(handle) => handle, + None => { + error!("Cannot allocate statement handle: no active connection"); + unsafe { *output_handle = std::ptr::null_mut() } + return SqlReturn::ERROR; + } + }; wrap_and_set(handle_type, handle, output_handle); info!("Successfully allocated a Stmt handle"); diff --git a/src/odbc/api/sqldescribecol.rs b/src/odbc/api/sqldescribecol.rs index 935f178..a478637 100644 --- a/src/odbc/api/sqldescribecol.rs +++ b/src/odbc/api/sqldescribecol.rs @@ -1,20 +1,134 @@ -use odbc_sys::SqlReturn; +use crate::odbc::implementation::alloc_handles::StatementHandle; +use crate::odbc::utils::get_from_wrapper; +use odbc_sys::{HandleType, SqlReturn}; use std::ffi::c_void; -use tracing::info; +use std::ptr; +use tracing::{debug, error, info}; +/// SQLDescribeColW returns the result descriptor for one column in the result set. +/// +/// This function provides column metadata including name, data type, size, +/// decimal digits, and nullability. It must be called after a statement has +/// been prepared (via SQLPrepareW or SQLExecDirectW). #[allow(non_snake_case)] #[unsafe(no_mangle)] -pub extern "C" fn SQLDescribeCol( - _statement_handle: *mut c_void, - _column_number: u16, - _column_name: *mut u16, - _buffer_length: i16, - _name_length_ptr: *mut i16, - _data_type_ptr: *mut i16, - _column_size_ptr: *mut usize, - _decimal_digits_ptr: *mut i16, - _nullable_ptr: *mut i16, +pub extern "C" fn SQLDescribeColW( + statement_handle: *mut c_void, + column_number: u16, + column_name: *mut u16, + buffer_length: i16, + name_length_ptr: *mut i16, + data_type_ptr: *mut i16, + column_size_ptr: *mut usize, + decimal_digits_ptr: *mut i16, + nullable_ptr: *mut i16, ) -> SqlReturn { - info!("SQLDescribeCol"); - SqlReturn::SUCCESS + info!( + "column_number={}, buffer_length={}", + column_number, buffer_length + ); + + // Get the statement handle + let statement_handle: &mut StatementHandle = + match get_from_wrapper(&HandleType::Stmt, statement_handle) { + Ok(handle) => handle, + Err(err) => { + error!("Failed to get statement handle: {}", err); + return SqlReturn::INVALID_HANDLE; + } + }; + + // Check if we have a prepared statement + let stmt = match &statement_handle.statement { + Some(stmt) => stmt, + None => { + error!("No prepared statement found"); + return SqlReturn::ERROR; + } + }; + + // Validate column number (1-indexed in ODBC) + if column_number == 0 || column_number as usize > stmt.column_count() { + error!( + "Invalid column number {}. Valid range: 1-{}", + column_number, + stmt.column_count() + ); + return SqlReturn::ERROR; + } + + // Convert to 0-indexed for SQLite + let col_index = (column_number - 1) as usize; + + // Write column name as UTF-16 + let mut result = SqlReturn::SUCCESS; + match stmt.column_name(col_index) { + Ok(name) => { + debug!("Column {} name: '{}'", column_number, name); + + let utf16_value: Vec = name.encode_utf16().collect(); + let utf16_len = utf16_value.len() as i16; + + // Set the actual name length in characters (excluding null terminator) + if !name_length_ptr.is_null() { + unsafe { + *name_length_ptr = utf16_len; + } + } + + // Copy the name string if buffer is provided + // buffer_length is in characters (u16 elements) per ODBC spec + if !column_name.is_null() && buffer_length > 0 { + let max_chars = buffer_length as usize; + let copy_len = std::cmp::min(utf16_value.len(), max_chars.saturating_sub(1)); + + unsafe { + ptr::copy_nonoverlapping(utf16_value.as_ptr(), column_name, copy_len); + // Null-terminate + *column_name.add(copy_len) = 0; + } + + if copy_len < utf16_value.len() { + result = SqlReturn::SUCCESS_WITH_INFO; + } + } + } + Err(err) => { + error!("Could not get column name for index {}: {}", col_index, err); + return SqlReturn::ERROR; + } + } + + // Set data type - SQLite is dynamically typed, report VARCHAR for all columns + if !data_type_ptr.is_null() { + unsafe { + *data_type_ptr = 12; // SQL_VARCHAR + } + debug!("Returning type: SQL_VARCHAR"); + } + + // Set column size - default VARCHAR length + if !column_size_ptr.is_null() { + unsafe { + *column_size_ptr = 255; + } + debug!("Returning column size: 255"); + } + + // Set decimal digits - 0 for VARCHAR + if !decimal_digits_ptr.is_null() { + unsafe { + *decimal_digits_ptr = 0; + } + } + + // Set nullable - SQLite columns are generally nullable + if !nullable_ptr.is_null() { + unsafe { + *nullable_ptr = 1; // SQL_NULLABLE + } + debug!("Returning nullable: SQL_NULLABLE"); + } + + result } diff --git a/src/odbc/api/sqltables.rs b/src/odbc/api/sqltables.rs index 0e4f850..0fff081 100644 --- a/src/odbc/api/sqltables.rs +++ b/src/odbc/api/sqltables.rs @@ -31,18 +31,30 @@ pub extern "C" fn SQLTablesW( } }; - // Assuming `statement_handle` is mutable and has fields for storing `stmt` and `rows` - let stmt = statement_handle + // Prepare the query to list tables + let stmt = match statement_handle .sqlite_connection .prepare("SELECT name FROM sqlite_master WHERE type='table'") - .unwrap(); + { + Ok(stmt) => stmt, + Err(err) => { + error!("Failed to prepare table listing query: {}", err); + return SqlReturn::ERROR; + } + }; statement_handle.statement = Some(stmt); if let Some(ref mut stmt) = statement_handle.statement { - statement_handle.rows = Some(stmt.query([]).unwrap()); + match stmt.query([]) { + Ok(rows) => { + statement_handle.rows = Some(rows); + } + Err(err) => { + error!("Failed to execute table listing query: {}", err); + return SqlReturn::ERROR; + } + } } - //let catalog_name = maybe_utf16_to_string(catalog_name, catalog_name_length).unwrap_or("DEFAULT".to_string()); - SqlReturn::SUCCESS } diff --git a/src/odbc/implementation/alloc_handles.rs b/src/odbc/implementation/alloc_handles.rs index 0120532..7df3809 100644 --- a/src/odbc/implementation/alloc_handles.rs +++ b/src/odbc/implementation/alloc_handles.rs @@ -41,12 +41,12 @@ pub(crate) fn impl_allocate_dbc_handle(_env_handle: &mut EnvironmentHandle) -> C pub(crate) fn allocate_stmt_handle( connection_handle: &mut ConnectionHandle, -) -> StatementHandle<'_> { - let connection_ref = connection_handle.sqlite_connection.as_ref().unwrap(); - StatementHandle { +) -> Option> { + let connection_ref = connection_handle.sqlite_connection.as_ref()?; + Some(StatementHandle { sqlite_connection: connection_ref, statement: None, rows: None, row: None, - } + }) } diff --git a/tests/basic_odbc_connection_test.rs b/tests/basic_odbc_connection_test.rs index 2da577b..cc3451d 100644 --- a/tests/basic_odbc_connection_test.rs +++ b/tests/basic_odbc_connection_test.rs @@ -1,6 +1,10 @@ #![allow(dead_code)] use odbc_api::buffers::TextRowSet; -use odbc_api::{ConnectionOptions, Cursor, Environment}; +use odbc_api::{ + ColumnDescription, ConnectionOptions, Cursor, DataType, Environment, Nullability, + ResultSetMetadata, +}; +use std::num::NonZeroUsize; use std::process::Command; /// Basic ODBC connection test using odbc-api crate @@ -136,6 +140,50 @@ fn test_multiple_connections() { println!("✅ Second connection closed successfully"); } +#[test] +fn test_describe_columns() { + let env = Environment::new().expect("Failed to create ODBC environment"); + + let connection = env + .connect("test_connection", "", "", ConnectionOptions::default()) + .expect("Failed to connect to database"); + + // Execute a query so we have a result set with known columns + let mut cursor = connection + .execute( + "SELECT name FROM sqlite_master WHERE type='table'", + (), + None, + ) + .expect("Failed to execute query") + .expect("Expected a cursor for SELECT statement"); + + // Describe column 1 (the "name" column) + let mut col_desc = ColumnDescription::default(); + cursor + .describe_col(1, &mut col_desc) + .expect("Failed to describe column 1"); + + let col_name = col_desc + .name_to_string() + .expect("Failed to decode column name"); + println!( + "Column 1 name: '{}', data_type: {:?}, nullable: {:?}", + col_name, col_desc.data_type, col_desc.nullability + ); + + assert_eq!(col_name, "name"); + assert_eq!( + col_desc.data_type, + DataType::Varchar { + length: NonZeroUsize::new(255) + } + ); + assert_eq!(col_desc.nullability, Nullability::Nullable); + + println!("describe_columns test passed"); +} + #[test] fn test_invalid_connection_string() { setup_test_environment().expect("Test environment setup failed"); From 7aa9aebd5dfca1acfcd6bb2f252bff2a45bda19f Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 13 Feb 2026 15:10:40 +0100 Subject: [PATCH 5/7] Normalize get_from_wrapper failures to return INVALID_HANDLE Per ODBC spec, SQL_INVALID_HANDLE should be returned when a handle argument is invalid (null, wrong type, or corrupted tag). Previously 8 of 16 call sites incorrectly returned SQL_ERROR instead. Also normalizes error messages to the consistent pattern: error!("Failed to get {handle_type} handle: {}", err) Co-Authored-By: Claude Opus 4.6 --- src/odbc/api/sqlallochandle.rs | 8 ++++---- src/odbc/api/sqlconnect.rs | 4 ++-- src/odbc/api/sqldriverconnect.rs | 4 ++-- src/odbc/api/sqlgetdata.rs | 4 ++-- src/odbc/api/sqlgetenvattr.rs | 4 ++-- src/odbc/api/sqlnumresultcols.rs | 4 ++-- src/odbc/api/sqlsetenvattr.rs | 4 ++-- src/odbc/api/sqltables.rs | 4 ++-- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/odbc/api/sqlallochandle.rs b/src/odbc/api/sqlallochandle.rs index 0db5123..9059cbf 100644 --- a/src/odbc/api/sqlallochandle.rs +++ b/src/odbc/api/sqlallochandle.rs @@ -73,9 +73,9 @@ pub extern "C" fn SQLAllocHandle( match get_from_wrapper(&HandleType::Env, input_handle) { Ok(env) => env, Err(err) => { - error!("Getting environment handle: {}", err); + error!("Failed to get environment handle: {}", err); unsafe { *output_handle = std::ptr::null_mut() } - return SqlReturn::ERROR; + return SqlReturn::INVALID_HANDLE; } }; @@ -91,9 +91,9 @@ pub extern "C" fn SQLAllocHandle( match get_from_wrapper(&HandleType::Dbc, input_handle) { Ok(env) => env, Err(err) => { - info!("Getting connection handle: {}", err); + error!("Failed to get connection handle: {}", err); unsafe { *output_handle = std::ptr::null_mut() } - return SqlReturn::ERROR; + return SqlReturn::INVALID_HANDLE; } }; diff --git a/src/odbc/api/sqlconnect.rs b/src/odbc/api/sqlconnect.rs index a0ccea9..3b8cca3 100644 --- a/src/odbc/api/sqlconnect.rs +++ b/src/odbc/api/sqlconnect.rs @@ -41,8 +41,8 @@ pub extern "C" fn SQLConnectW( match get_from_wrapper(&HandleType::Dbc, connection_handle) { Ok(env) => env, Err(e) => { - error!("{}", e); - return SqlReturn::ERROR; + error!("Failed to get connection handle: {}", e); + return SqlReturn::INVALID_HANDLE; } }; diff --git a/src/odbc/api/sqldriverconnect.rs b/src/odbc/api/sqldriverconnect.rs index 276ee1a..48e93d9 100644 --- a/src/odbc/api/sqldriverconnect.rs +++ b/src/odbc/api/sqldriverconnect.rs @@ -50,8 +50,8 @@ pub extern "C" fn SQLDriverConnectW( match get_from_wrapper(&HandleType::Dbc, connection_handle) { Ok(conn) => conn, Err(e) => { - error!("Error getting connection handle {}", e); - return SqlReturn::ERROR; + error!("Failed to get connection handle: {}", e); + return SqlReturn::INVALID_HANDLE; } }; diff --git a/src/odbc/api/sqlgetdata.rs b/src/odbc/api/sqlgetdata.rs index 5a912b7..acdfb28 100644 --- a/src/odbc/api/sqlgetdata.rs +++ b/src/odbc/api/sqlgetdata.rs @@ -21,8 +21,8 @@ pub extern "C" fn SQLGetData( match get_from_wrapper(&HandleType::Stmt, statement_handle) { Ok(handle) => handle, Err(err) => { - error!("{}", err); - return SqlReturn::ERROR; + error!("Failed to get statement handle: {}", err); + return SqlReturn::INVALID_HANDLE; } }; diff --git a/src/odbc/api/sqlgetenvattr.rs b/src/odbc/api/sqlgetenvattr.rs index 6284707..b166169 100644 --- a/src/odbc/api/sqlgetenvattr.rs +++ b/src/odbc/api/sqlgetenvattr.rs @@ -38,8 +38,8 @@ pub extern "C" fn SQLGetEnvAttr( let env: &mut EnvironmentHandle = match get_from_wrapper(&HandleType::Env, environment_handle) { Ok(env) => env, Err(err) => { - error!("{}", err); - return SqlReturn::ERROR; + error!("Failed to get environment handle: {}", err); + return SqlReturn::INVALID_HANDLE; } }; diff --git a/src/odbc/api/sqlnumresultcols.rs b/src/odbc/api/sqlnumresultcols.rs index ec68fcb..d1c00d3 100644 --- a/src/odbc/api/sqlnumresultcols.rs +++ b/src/odbc/api/sqlnumresultcols.rs @@ -27,8 +27,8 @@ pub unsafe extern "C" fn SQLNumResultCols( match get_from_wrapper(&HandleType::Stmt, statement_handle) { Ok(handle) => handle, Err(err) => { - error!("{}", err); - return SqlReturn::ERROR; + error!("Failed to get statement handle: {}", err); + return SqlReturn::INVALID_HANDLE; } }; diff --git a/src/odbc/api/sqlsetenvattr.rs b/src/odbc/api/sqlsetenvattr.rs index 28d26a0..cec92b1 100644 --- a/src/odbc/api/sqlsetenvattr.rs +++ b/src/odbc/api/sqlsetenvattr.rs @@ -38,8 +38,8 @@ pub fn SQLSetEnvAttr( let env: &mut EnvironmentHandle = match get_from_wrapper(&HandleType::Env, environment_handle) { Ok(env) => env, Err(err) => { - error!("{}", err); - return SqlReturn::ERROR; + error!("Failed to get environment handle: {}", err); + return SqlReturn::INVALID_HANDLE; } }; diff --git a/src/odbc/api/sqltables.rs b/src/odbc/api/sqltables.rs index 0fff081..be78056 100644 --- a/src/odbc/api/sqltables.rs +++ b/src/odbc/api/sqltables.rs @@ -26,8 +26,8 @@ pub extern "C" fn SQLTablesW( match get_from_wrapper(&HandleType::Stmt, statement_handle) { Ok(env) => env, Err(err) => { - error!("{}", err); - return SqlReturn::ERROR; + error!("Failed to get statement handle: {}", err); + return SqlReturn::INVALID_HANDLE; } }; From b493a6feacc24f3042701668aa99ba1a663c230c Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 17 Mar 2026 12:40:29 +0100 Subject: [PATCH 6/7] minor cleanup --- tests/basic_odbc_connection_test.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/basic_odbc_connection_test.rs b/tests/basic_odbc_connection_test.rs index cc3451d..14369ad 100644 --- a/tests/basic_odbc_connection_test.rs +++ b/tests/basic_odbc_connection_test.rs @@ -13,7 +13,6 @@ use std::process::Command; /// to our driver through the standard ODBC client stack. const CONNECTION_STRING: &str = "DSN=test_connection"; -//const CONNECTION_STRING: &str = "Driver=/home/andrew/gitrepos/odbc-rs-sqlite/target/debug/libodbc_driver_rs.so;Database=/home/andrew/gitrepos/odbc-rs-sqlite/test_odbc.sqlite"; /// Set up test environment by building driver and configuring ODBC fn setup_test_environment() -> std::result::Result<(), Box> { From 757b7f5046d7af146a9d0402c25a10fd0b6a2003 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 17 Mar 2026 14:59:04 +0100 Subject: [PATCH 7/7] implement SQLColumnsW --- src/odbc/api/sqlcolumns.rs | 41 ++++- src/odbc/api/sqltables.rs | 30 +--- src/odbc/implementation.rs | 2 + src/odbc/implementation/columns.rs | 257 +++++++++++++++++++++++++++++ src/odbc/implementation/tables.rs | 100 +++++++++++ 5 files changed, 400 insertions(+), 30 deletions(-) create mode 100644 src/odbc/implementation/columns.rs create mode 100644 src/odbc/implementation/tables.rs diff --git a/src/odbc/api/sqlcolumns.rs b/src/odbc/api/sqlcolumns.rs index d72e80f..e344077 100644 --- a/src/odbc/api/sqlcolumns.rs +++ b/src/odbc/api/sqlcolumns.rs @@ -1,20 +1,49 @@ -use odbc_sys::SqlReturn; +use crate::odbc::implementation::alloc_handles::StatementHandle; +use crate::odbc::implementation::columns::impl_get_columns; +use crate::odbc::utils::{get_from_wrapper, maybe_utf16_to_string}; +use odbc_sys::{HandleType, SqlReturn}; use std::ffi::c_void; -use tracing::info; +use tracing::{debug, error, info}; #[allow(non_snake_case)] #[unsafe(no_mangle)] pub extern "C" fn SQLColumnsW( - _statement_handle: *mut c_void, + statement_handle: *mut c_void, _catalog_name: *const u16, _catalog_name_length: i16, _schema_name: *const u16, _schema_name_length: i16, - _table_name: *const u16, - _table_name_length: i16, + table_name: *const u16, + table_name_length: i16, _column_name: *const u16, _column_name_length: i16, ) -> SqlReturn { info!("SQLColumnsW"); - SqlReturn::SUCCESS + + let statement_handle: &mut StatementHandle = + match get_from_wrapper(&HandleType::Stmt, statement_handle) { + Ok(handle) => handle, + Err(err) => { + error!("Failed to get statement handle: {}", err); + return SqlReturn::INVALID_HANDLE; + } + }; + + let table_name = match maybe_utf16_to_string(table_name, table_name_length) { + Some(name) => name, + None => { + error!("Table name is required for SQLColumnsW"); + return SqlReturn::ERROR; + } + }; + + debug!("Getting columns for table: {}", table_name); + + match impl_get_columns(statement_handle, &table_name) { + Ok(()) => SqlReturn::SUCCESS, + Err(err) => { + error!("impl_get_columns failed: {}", err); + SqlReturn::ERROR + } + } } diff --git a/src/odbc/api/sqltables.rs b/src/odbc/api/sqltables.rs index be78056..43b2c85 100644 --- a/src/odbc/api/sqltables.rs +++ b/src/odbc/api/sqltables.rs @@ -1,4 +1,5 @@ use crate::odbc::implementation::alloc_handles::StatementHandle; +use crate::odbc::implementation::tables::impl_get_tables; use crate::odbc::utils::get_from_wrapper; use odbc_sys::{HandleType, SqlReturn}; use std::ffi::c_void; @@ -24,37 +25,18 @@ pub extern "C" fn SQLTablesW( let statement_handle: &mut StatementHandle = match get_from_wrapper(&HandleType::Stmt, statement_handle) { - Ok(env) => env, + Ok(handle) => handle, Err(err) => { error!("Failed to get statement handle: {}", err); return SqlReturn::INVALID_HANDLE; } }; - // Prepare the query to list tables - let stmt = match statement_handle - .sqlite_connection - .prepare("SELECT name FROM sqlite_master WHERE type='table'") - { - Ok(stmt) => stmt, + match impl_get_tables(statement_handle) { + Ok(()) => SqlReturn::SUCCESS, Err(err) => { - error!("Failed to prepare table listing query: {}", err); - return SqlReturn::ERROR; - } - }; - statement_handle.statement = Some(stmt); - - if let Some(ref mut stmt) = statement_handle.statement { - match stmt.query([]) { - Ok(rows) => { - statement_handle.rows = Some(rows); - } - Err(err) => { - error!("Failed to execute table listing query: {}", err); - return SqlReturn::ERROR; - } + error!("impl_get_tables failed: {}", err); + SqlReturn::ERROR } } - - SqlReturn::SUCCESS } diff --git a/src/odbc/implementation.rs b/src/odbc/implementation.rs index c9d14b1..e196288 100644 --- a/src/odbc/implementation.rs +++ b/src/odbc/implementation.rs @@ -1,6 +1,8 @@ pub(crate) mod alloc_handles; +pub(crate) mod columns; pub(crate) mod connect; pub(crate) mod env_attrs; pub(crate) mod getdata; #[allow(clippy::module_inception)] pub(crate) mod implementation; +pub(crate) mod tables; diff --git a/src/odbc/implementation/columns.rs b/src/odbc/implementation/columns.rs new file mode 100644 index 0000000..f206c74 --- /dev/null +++ b/src/odbc/implementation/columns.rs @@ -0,0 +1,257 @@ +use crate::odbc::implementation::alloc_handles::StatementHandle; +use tracing::error; + +pub(crate) fn impl_get_columns<'a>( + statement_handle: &mut StatementHandle<'a>, + table_name: &str, +) -> Result<(), String> { + // Validate table name before interpolating into SQL. + // pragma_table_info does not accept bound parameters, so we must + // ensure the name contains only safe characters. + if !table_name.chars().all(|c| c.is_alphanumeric() || c == '_') { + return Err(format!("Invalid table name: '{table_name}'")); + } + + // pragma_table_info('') is a SQLite table-valued function that returns + // one row per column: cid, name, type, notnull, dflt_value, pk. + // + // We reshape the output into the 18-column result set required by the ODBC + // spec for SQLColumns. Columns that SQLite cannot provide are returned as NULL. + // + // ODBC SQL type codes used here: + // -7 = SQL_BIT, 2 = SQL_NUMERIC, 4 = SQL_INTEGER, + // 8 = SQL_DOUBLE, -4 = SQL_LONGVARBINARY, 12 = SQL_VARCHAR (default) + // Column names from pragma_table_info that are SQLite reserved words must be + // double-quoted: "notnull" and "type". Others ("name", "cid", "dflt_value") are + // quoted for consistency. + let sql = format!( + r#"SELECT + NULL AS TABLE_CAT, + NULL AS TABLE_SCHEM, + '{table_name}' AS TABLE_NAME, + "name" AS COLUMN_NAME, + CASE + WHEN upper("type") LIKE '%INT%' THEN 4 + WHEN upper("type") LIKE '%REAL%' + OR upper("type") LIKE '%FLOAT%' + OR upper("type") LIKE '%DOUBLE%' THEN 8 + WHEN upper("type") LIKE '%BLOB%' THEN -4 + WHEN upper("type") LIKE '%BOOL%' THEN -7 + WHEN upper("type") LIKE '%NUMERIC%' + OR upper("type") LIKE '%DECIMAL%' THEN 2 + ELSE 12 + END AS DATA_TYPE, + CASE WHEN "type" = '' THEN 'TEXT' ELSE "type" END AS TYPE_NAME, + NULL AS COLUMN_SIZE, + NULL AS BUFFER_LENGTH, + NULL AS DECIMAL_DIGITS, + NULL AS NUM_PREC_RADIX, + CASE WHEN "notnull" = 1 THEN 0 ELSE 1 END AS NULLABLE, + NULL AS REMARKS, + "dflt_value" AS COLUMN_DEF, + CASE + WHEN upper("type") LIKE '%INT%' THEN 4 + WHEN upper("type") LIKE '%REAL%' + OR upper("type") LIKE '%FLOAT%' + OR upper("type") LIKE '%DOUBLE%' THEN 8 + WHEN upper("type") LIKE '%BLOB%' THEN -4 + WHEN upper("type") LIKE '%BOOL%' THEN -7 + WHEN upper("type") LIKE '%NUMERIC%' + OR upper("type") LIKE '%DECIMAL%' THEN 2 + ELSE 12 + END AS SQL_DATA_TYPE, + NULL AS SQL_DATETIME_SUB, + NULL AS CHAR_OCTET_LENGTH, + "cid" + 1 AS ORDINAL_POSITION, + CASE WHEN "notnull" = 1 THEN 'NO' ELSE 'YES' END AS IS_NULLABLE + FROM pragma_table_info('{table_name}')"# + ); + + let stmt = statement_handle + .sqlite_connection + .prepare(&sql) + .map_err(|e| { + error!("Failed to prepare column query for '{}': {}", table_name, e); + e.to_string() + })?; + + statement_handle.statement = Some(stmt); + + let rows = statement_handle + .statement + .as_mut() + .unwrap() + .query([]) + .map_err(|e| { + error!("Failed to execute column query for '{}': {}", table_name, e); + e.to_string() + })?; + + // Safety: `rows` borrows from `statement_handle.statement` which has lifetime `'a`. + // The borrow checker infers a shorter lifetime for the reference, but the data lives + // for `'a`. We transmute here to express that — the same pattern the rest of this + // codebase relies on implicitly via `get_from_wrapper`. + let rows: rusqlite::Rows<'a> = unsafe { std::mem::transmute(rows) }; + statement_handle.rows = Some(rows); + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::odbc::implementation::alloc_handles::StatementHandle; + use rusqlite::Connection; + + fn make_test_db() -> Connection { + let conn = Connection::open_in_memory().unwrap(); + conn.execute_batch( + "CREATE TABLE widgets ( + widget_id INTEGER PRIMARY KEY, + name VARCHAR(50) NOT NULL, + weight REAL, + notes TEXT + );", + ) + .unwrap(); + conn + } + + #[test] + fn result_set_has_18_columns() { + let conn = make_test_db(); + let mut handle = StatementHandle { + sqlite_connection: &conn, + statement: None, + rows: None, + row: None, + }; + + impl_get_columns(&mut handle, "widgets").unwrap(); + + let col_count = handle.statement.as_ref().unwrap().column_count(); + assert_eq!(col_count, 18); + } + + #[test] + fn result_set_has_one_row_per_table_column() { + let conn = make_test_db(); + let mut handle = StatementHandle { + sqlite_connection: &conn, + statement: None, + rows: None, + row: None, + }; + + impl_get_columns(&mut handle, "widgets").unwrap(); + + let mut count = 0; + let rows = handle.rows.as_mut().unwrap(); + while rows.next().unwrap().is_some() { + count += 1; + } + assert_eq!(count, 4); // widgets has 4 columns + } + + #[test] + fn rows_contain_correct_table_and_column_names() { + let conn = make_test_db(); + let mut handle = StatementHandle { + sqlite_connection: &conn, + statement: None, + rows: None, + row: None, + }; + + impl_get_columns(&mut handle, "widgets").unwrap(); + + let rows = handle.rows.as_mut().unwrap(); + let row = rows.next().unwrap().unwrap(); + + // Column 3 (index 2) = TABLE_NAME, column 4 (index 3) = COLUMN_NAME + let table_name: String = row.get(2).unwrap(); + let column_name: String = row.get(3).unwrap(); + + assert_eq!(table_name, "widgets"); + assert_eq!(column_name, "widget_id"); + } + + #[test] + fn ordinal_position_is_one_based() { + let conn = make_test_db(); + let mut handle = StatementHandle { + sqlite_connection: &conn, + statement: None, + rows: None, + row: None, + }; + + impl_get_columns(&mut handle, "widgets").unwrap(); + + let rows = handle.rows.as_mut().unwrap(); + let row = rows.next().unwrap().unwrap(); + + // Column 17 (index 16) = ORDINAL_POSITION + let ordinal: i64 = row.get(16).unwrap(); + assert_eq!(ordinal, 1); + } + + #[test] + fn not_null_column_has_nullable_zero() { + let conn = make_test_db(); + let mut handle = StatementHandle { + sqlite_connection: &conn, + statement: None, + rows: None, + row: None, + }; + + impl_get_columns(&mut handle, "widgets").unwrap(); + + let rows = handle.rows.as_mut().unwrap(); + + // widget_id is PRIMARY KEY (implicitly NOT NULL); skip to name (NOT NULL) + rows.next().unwrap(); // widget_id + let row = rows.next().unwrap().unwrap(); // name VARCHAR(50) NOT NULL + + // Column 11 (index 10) = NULLABLE: 0 = not nullable + let nullable: i64 = row.get(10).unwrap(); + assert_eq!(nullable, 0); + } + + #[test] + fn nullable_column_has_nullable_one() { + let conn = make_test_db(); + let mut handle = StatementHandle { + sqlite_connection: &conn, + statement: None, + rows: None, + row: None, + }; + + impl_get_columns(&mut handle, "widgets").unwrap(); + + let rows = handle.rows.as_mut().unwrap(); + rows.next().unwrap(); // widget_id + rows.next().unwrap(); // name + let row = rows.next().unwrap().unwrap(); // weight REAL (nullable) + + // Column 11 (index 10) = NULLABLE: 1 = nullable + let nullable: i64 = row.get(10).unwrap(); + assert_eq!(nullable, 1); + } + + #[test] + fn rejects_table_name_with_special_characters() { + let conn = make_test_db(); + let mut handle = StatementHandle { + sqlite_connection: &conn, + statement: None, + rows: None, + row: None, + }; + + let result = impl_get_columns(&mut handle, "widgets; DROP TABLE widgets"); + assert!(result.is_err()); + } +} diff --git a/src/odbc/implementation/tables.rs b/src/odbc/implementation/tables.rs new file mode 100644 index 0000000..56d5d5e --- /dev/null +++ b/src/odbc/implementation/tables.rs @@ -0,0 +1,100 @@ +use crate::odbc::implementation::alloc_handles::StatementHandle; + +pub(crate) fn impl_get_tables<'a>( + statement_handle: &mut StatementHandle<'a>, +) -> Result<(), String> { + let stmt = statement_handle + .sqlite_connection + .prepare("SELECT name FROM sqlite_master WHERE type='table'") + .map_err(|e| e.to_string())?; + + statement_handle.statement = Some(stmt); + + let rows = statement_handle + .statement + .as_mut() + .unwrap() + .query([]) + .map_err(|e| e.to_string())?; + + // Safety: `rows` borrows from `statement_handle.statement` which has lifetime `'a`. + // We transmute the inferred shorter reference lifetime to `'a` — the same pattern + // used throughout this codebase via `get_from_wrapper`. + let rows: rusqlite::Rows<'a> = unsafe { std::mem::transmute(rows) }; + statement_handle.rows = Some(rows); + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::odbc::implementation::alloc_handles::StatementHandle; + use rusqlite::Connection; + + fn make_test_db() -> Connection { + let conn = Connection::open_in_memory().unwrap(); + conn.execute_batch( + "CREATE TABLE apples (id INTEGER PRIMARY KEY); + CREATE TABLE oranges (id INTEGER PRIMARY KEY);", + ) + .unwrap(); + conn + } + + #[test] + fn result_set_is_populated_after_call() { + let conn = make_test_db(); + let mut handle = StatementHandle { + sqlite_connection: &conn, + statement: None, + rows: None, + row: None, + }; + + let result = impl_get_tables(&mut handle); + + assert!(result.is_ok()); + assert!(handle.statement.is_some()); + assert!(handle.rows.is_some()); + } + + #[test] + fn returns_one_row_per_table() { + let conn = make_test_db(); + let mut handle = StatementHandle { + sqlite_connection: &conn, + statement: None, + rows: None, + row: None, + }; + + impl_get_tables(&mut handle).unwrap(); + + let mut count = 0; + let rows = handle.rows.as_mut().unwrap(); + while rows.next().unwrap().is_some() { + count += 1; + } + // sqlite_sequence is also present (autoincrement tracking table) + assert!(count >= 2); + } + + #[test] + fn row_contains_table_name() { + let conn = make_test_db(); + let mut handle = StatementHandle { + sqlite_connection: &conn, + statement: None, + rows: None, + row: None, + }; + + impl_get_tables(&mut handle).unwrap(); + + let rows = handle.rows.as_mut().unwrap(); + let row = rows.next().unwrap().unwrap(); + let name: String = row.get(0).unwrap(); + assert!(!name.is_empty()); + } +}