diff --git a/CHANGELOG.md b/CHANGELOG.md index abc5d82..fe59fb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- `SQLGetTypeInfo` lists `TEXT` before `VARCHAR` for `SQL_VARCHAR`, as the + spec's "how closely the data type maps" ordering requires: `TEXT` is SQLite's + own storage class for character data. The rows were sorted by name, so an + application taking the first row of a `DATA_TYPE` as its `CAST` target, as + Power Query does, got `VARCHAR`. The type names reported for columns are + unchanged. + ## [0.1.0] — 2026-08-05 First release, so this section describes what the driver offers rather than diff --git a/Cargo.lock b/Cargo.lock index 81a7500..8101537 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -858,8 +858,8 @@ dependencies = [ [[package]] name = "stackable-odbc-core" -version = "0.1.0" -source = "git+https://github.com/stackabletech/stackable-odbc-core.git?tag=v0.1.0#23c924489e135d1d3da1d1664ae16bf8656d5aa3" +version = "0.1.1" +source = "git+https://github.com/stackabletech/stackable-odbc-core.git?tag=v0.1.1#4f5e802cf0cded7c75dba8afa905864b9f355302" dependencies = [ "odbc-sys", "snafu", diff --git a/Cargo.toml b/Cargo.toml index 0b6f1fa..04341df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,7 @@ rusqlite = { version = "0.40", features = [ ] } serde_json = "1" snafu = "0.9" -stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", tag = "v0.1.0" } +stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", tag = "v0.1.1" } tracing = "0.1" [dev-dependencies] @@ -41,7 +41,7 @@ proptest = "1" # attach/detach helpers. Default-off there because it is test code that would # otherwise land in this driver's shipped binary; enabled only here, so # `cargo test` sees it and `cargo build` does not. -stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", tag = "v0.1.0", features = ["test-support"] } +stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", tag = "v0.1.1", features = ["test-support"] } [lints.clippy] unwrap_in_result = "deny" diff --git a/src/backend/info.rs b/src/backend/info.rs index 55ca62d..bfcdc5a 100644 --- a/src/backend/info.rs +++ b/src/backend/info.rs @@ -115,12 +115,12 @@ static SUPPORTED_FUNCTIONS: &[FunctionId] = &[ // maximum supported precision/scale) rather than hand-written (see // `stackable_odbc_core::types::column_size` module docs). // -// Rows are sorted by DATA_TYPE ascending (as signed i16, so ODBC extension -// types with negative codes sort first), then by TYPE_NAME ascending within -// an equal DATA_TYPE, per the SQLGetTypeInfo spec's "ordered by DATA_TYPE and -// then ... TYPE_NAME" requirement. This invariant is asserted directly by -// `type_info_rows_sorted_by_data_type_then_type_name` below; keep new rows -// in the correct sorted position rather than appending them. +// Core orders the result set itself (DATA_TYPE, then the row marked +// `with_preferred`, then TYPE_NAME; see +// `stackable_odbc_core::ffi::info::sql_get_type_info`), so rows are grouped +// here for reading, not for the spec. Every DATA_TYPE shared by several rows +// marks its closest match, as `every_shared_data_type_has_one_preferred_row` +// asserts. // // A `LazyLock` rather than a plain `static`: `TypeInfoRow`'s string fields are // `Cow<'static, str>` so a backend can compute them, and converting a `&'static @@ -260,6 +260,10 @@ static SQLITE_TYPE_INFO: std::sync::LazyLock> = std::sync::Lazy // the same size. 255 is the value the rest of the driver treats as // authoritative for this DATA_TYPE (`default_precision_for_type`, and the // WVARCHAR row below), so both rows use it. + // + // Preferred over the VARCHAR row below: TEXT is SQLite's own storage + // class, and SQLite ignores a VARCHAR(n) length, so it is the closer + // match for SQL_VARCHAR. Core orders it first among the two. TypeInfoRow::new("TEXT", SqlDataType::VARCHAR) .with_column_size(catalog_column_size( SqlDataType::VARCHAR, @@ -268,7 +272,8 @@ static SQLITE_TYPE_INFO: std::sync::LazyLock> = std::sync::Lazy )) .with_literal_affixes(Some("'"), Some("'")) .with_create_params(Some("max length")) - .with_case_sensitive(true), + .with_case_sensitive(true) + .with_preferred(true), // SQL_VARCHAR (12): ANSI alias needed for Windows DM / pyodbc type // conversion (AGENTS.md "Windows Driver Manager compatibility // checklist"). sqlite_type_to_sql_data_type never actually returns this @@ -905,12 +910,14 @@ pub(super) fn get_type_info() -> &'static [TypeInfoRow] { /// /// `SQLITE_TYPE_INFO` has two rows sharing `DATA_TYPE=SqlDataType::VARCHAR` /// (`"TEXT"` and `"VARCHAR"`, both kept purely for Windows DM/pyodbc ANSI -/// compatibility; see the `WVARCHAR`/`TEXT`/`VARCHAR` row comments above), -/// with no single correct bare name for that DATA_TYPE. That case is -/// rejected explicitly below rather than left to `.find` picking whichever -/// row happens to come first: `sqlite_type_to_sql_data_type` never actually -/// returns the ANSI code (`SqlDataType::VARCHAR`) for any declared type -/// today, so the ambiguity is currently unreachable, but this function no +/// compatibility; see the `WVARCHAR`/`TEXT`/`VARCHAR` row comments above). +/// `TEXT` is marked preferred, so it is the row `SQLGetTypeInfo` lists first, +/// but which name a *column* should carry is a separate question: a column +/// declared `VARCHAR(50)` is not honestly reported as `TEXT`. That case is +/// therefore rejected explicitly below rather than left to `.find` picking +/// whichever row happens to come first: `sqlite_type_to_sql_data_type` never +/// actually returns the ANSI code (`SqlDataType::VARCHAR`) for any declared +/// type today, so the question is currently unreachable, but this function no /// longer depends on that fact staying true to give a correct answer; it /// would rather report "unknown" than silently guess. pub(super) fn sqlite_bare_type_name(sql_type: SqlDataType) -> &'static str { @@ -2284,35 +2291,30 @@ mod tests { ); } + /// Core orders the result set (DATA_TYPE, preferred row, TYPE_NAME), so the + /// declaration order here no longer matters. What does matter is that every + /// DATA_TYPE shared by several rows names its closest match, rather than + /// leaving the first row to the alphabet. #[test] - fn type_info_rows_sorted_by_data_type_then_type_name() { - // Spec (SQLGetTypeInfo): "ordered by DATA_TYPE and then ... TYPE_NAME, - // both ascending." DATA_TYPE is a signed i16 (negative for ODBC - // extension types), so the comparison must not treat it as unsigned. - // This walks adjacent pairs rather than asserting a fixed sequence, - // so it keeps holding as rows are added or reordered. - for pair in SQLITE_TYPE_INFO.windows(2) { - let (prev, next) = (&pair[0], &pair[1]); - assert!( - prev.data_type().0 <= next.data_type().0, - "SQLITE_TYPE_INFO not sorted by DATA_TYPE: {:?} (DATA_TYPE={}) \ - appears before {:?} (DATA_TYPE={})", - prev.type_name(), - prev.data_type().0, - next.type_name(), - next.data_type().0 - ); - if prev.data_type() == next.data_type() { - assert!( - prev.type_name() <= next.type_name(), - "rows sharing DATA_TYPE={} not sorted by TYPE_NAME: {:?} appears \ - before {:?}", - prev.data_type().0, - prev.type_name(), - next.type_name() - ); - } - } + fn every_shared_data_type_has_one_preferred_row() { + let issues = + stackable_odbc_core::conformance::type_info_preference_issues(&SQLITE_TYPE_INFO); + assert!( + issues.is_empty(), + "SQLGetTypeInfo preference markers: {issues:#?}" + ); + } + + /// The specific choice: `TEXT`, SQLite's own storage class, over its + /// `VARCHAR` alias for SQL_VARCHAR. + #[test] + fn sql_varchar_prefers_text() { + let preferred: Vec<&str> = SQLITE_TYPE_INFO + .iter() + .filter(|r| r.preferred()) + .map(|r| r.type_name()) + .collect(); + assert_eq!(preferred, vec!["TEXT"]); } /// Guards that SQL_DRIVER_VER is derived from the crate version rather diff --git a/src/ffi_integration_tests.rs b/src/ffi_integration_tests.rs index a67a8c4..6eda81d 100644 --- a/src/ffi_integration_tests.rs +++ b/src/ffi_integration_tests.rs @@ -861,6 +861,33 @@ fn get_type_info_filters_by_data_type() { } } +/// SQL_VARCHAR is the one DATA_TYPE with two rows (TEXT and VARCHAR). An +/// application taking the first row for a DATA_TYPE as its type gets the +/// driver's preferred one, through core's real entry point. +#[test] +fn get_type_info_leads_sql_varchar_with_text() { + unsafe { + let (env, conn, stmt) = alloc_handles(); + assert_eq!(connect_memory(conn), SqlReturn::SUCCESS); + + let ret = ffi::info::sql_get_type_info::(stmt, SqlDataType::VARCHAR.0); + assert_eq!(ret, SqlReturn::SUCCESS); + + let mut names = Vec::new(); + loop { + let ret = ffi::fetch::sql_fetch::(stmt); + if ret == SqlReturn::NO_DATA { + break; + } + assert_eq!(ret, SqlReturn::SUCCESS); + names.push(fetch_string_col(stmt, 1)); + } + assert_eq!(names, vec!["TEXT", "VARCHAR"]); + + cleanup(env, conn, stmt); + } +} + #[test] fn row_count_after_exec_direct() { unsafe {