From 513724e6b45bac5eb08388b0ca03259b7da48fe2 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Tue, 6 Oct 2026 17:53:35 +0200 Subject: [PATCH 1/5] fix: mark TEXT as the preferred SQL_VARCHAR type-info row --- Cargo.lock | 2 +- Cargo.toml | 7 ++-- src/backend/info.rs | 70 ++++++++++++++++++------------------ src/ffi_integration_tests.rs | 27 ++++++++++++++ 4 files changed, 68 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 81a7500..d885d59 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -859,7 +859,7 @@ 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" +source = "git+https://github.com/stackabletech/stackable-odbc-core.git?rev=23f055e451cde6b842ad117f5c8a16957abe113f#23f055e451cde6b842ad117f5c8a16957abe113f" dependencies = [ "odbc-sys", "snafu", diff --git a/Cargo.toml b/Cargo.toml index 0b6f1fa..3895ead 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,10 @@ 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" } +# TEMPORARY: pinned to the core commit adding `TypeInfoRow::with_preferred` +# until a core release containing it is tagged; switch both entries back to +# that tag before merging. +stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", rev = "23f055e451cde6b842ad117f5c8a16957abe113f" } tracing = "0.1" [dev-dependencies] @@ -41,7 +44,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", rev = "23f055e451cde6b842ad117f5c8a16957abe113f", features = ["test-support"] } [lints.clippy] unwrap_in_result = "deny" diff --git a/src/backend/info.rs b/src/backend/info.rs index 55ca62d..380a4e1 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 @@ -2284,35 +2289,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 { From 7d2ca25757a6f78beab45d87a3b989a0eb03025b Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Wed, 7 Oct 2026 15:16:00 +0200 Subject: [PATCH 2/5] docs: reconcile sqlite_bare_type_name's comment with the preferred TEXT row TEXT now leads SQLGetTypeInfo for SQL_VARCHAR, but a column's own name is a separate question: a column declared VARCHAR(50) is not honestly reported as TEXT, so the function still answers "" for SQL_VARCHAR. Comment only. --- src/backend/info.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/backend/info.rs b/src/backend/info.rs index 380a4e1..bfcdc5a 100644 --- a/src/backend/info.rs +++ b/src/backend/info.rs @@ -910,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 { From 151ff5e65d5b0504d9748c30c8e65e2b0b598015 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Thu, 8 Oct 2026 17:46:50 +0200 Subject: [PATCH 3/5] docs: add a changelog entry for the preferred TEXT type-info row 513724e changed the order SQLGetTypeInfo reports for SQL_VARCHAR (TEXT now before VARCHAR), which an application can observe, but landed without a changelog entry. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) 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 From 57e5aca3e6a0c8540bfa82500ac22c650c5668b4 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 9 Oct 2026 11:10:02 +0200 Subject: [PATCH 4/5] chore: pin stackable-odbc-core to 53e5a89 Moves the temporary pin from 23f055e (the preferred SQLGetTypeInfo row) to the head of core's fix/review-feedback branch, the same commit the Trino driver pins. The later core commits add the character and exact numeric -> SQL_C_INTERVAL_* conversions and 22015 for oversized interval fields; none needs SQLite changes. Still temporary until a core release is tagged. Co-Authored-By: Claude Opus 5.5 (1M context) --- Cargo.lock | 2 +- Cargo.toml | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d885d59..a8e96da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -859,7 +859,7 @@ dependencies = [ [[package]] name = "stackable-odbc-core" version = "0.1.0" -source = "git+https://github.com/stackabletech/stackable-odbc-core.git?rev=23f055e451cde6b842ad117f5c8a16957abe113f#23f055e451cde6b842ad117f5c8a16957abe113f" +source = "git+https://github.com/stackabletech/stackable-odbc-core.git?rev=53e5a89851fcfeeae404b9ab0ca461f149a0ff2b#53e5a89851fcfeeae404b9ab0ca461f149a0ff2b" dependencies = [ "odbc-sys", "snafu", diff --git a/Cargo.toml b/Cargo.toml index 3895ead..d87be86 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,10 +31,11 @@ rusqlite = { version = "0.40", features = [ ] } serde_json = "1" snafu = "0.9" -# TEMPORARY: pinned to the core commit adding `TypeInfoRow::with_preferred` -# until a core release containing it is tagged; switch both entries back to -# that tag before merging. -stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", rev = "23f055e451cde6b842ad117f5c8a16957abe113f" } +# TEMPORARY: pinned to the head of core's fix/review-feedback branch (the +# preferred SQLGetTypeInfo row and the interval conversions) until a core +# release containing it is tagged; switch both entries back to that tag before +# merging. +stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", rev = "53e5a89851fcfeeae404b9ab0ca461f149a0ff2b" } tracing = "0.1" [dev-dependencies] @@ -44,7 +45,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", rev = "23f055e451cde6b842ad117f5c8a16957abe113f", features = ["test-support"] } +stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", rev = "53e5a89851fcfeeae404b9ab0ca461f149a0ff2b", features = ["test-support"] } [lints.clippy] unwrap_in_result = "deny" From 75153e804eff9db42a5eb760bd6b51813919b508 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 9 Oct 2026 12:31:36 +0200 Subject: [PATCH 5/5] chore: pin stackable-odbc-core to the v0.1.1 release Replaces the temporary commit pin with the released tag. v0.1.1 is the squash-merged core work this branch was tested against (53e5a89 plus the version bump and changelog), so the dependency's code is unchanged; the lockfile records core 0.1.1 at tag v0.1.1 (4f5e802). Co-Authored-By: Claude Opus 5.5 (1M context) --- Cargo.lock | 4 ++-- Cargo.toml | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a8e96da..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?rev=53e5a89851fcfeeae404b9ab0ca461f149a0ff2b#53e5a89851fcfeeae404b9ab0ca461f149a0ff2b" +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 d87be86..04341df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,11 +31,7 @@ rusqlite = { version = "0.40", features = [ ] } serde_json = "1" snafu = "0.9" -# TEMPORARY: pinned to the head of core's fix/review-feedback branch (the -# preferred SQLGetTypeInfo row and the interval conversions) until a core -# release containing it is tagged; switch both entries back to that tag before -# merging. -stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", rev = "53e5a89851fcfeeae404b9ab0ca461f149a0ff2b" } +stackable-odbc-core = { git = "https://github.com/stackabletech/stackable-odbc-core.git", tag = "v0.1.1" } tracing = "0.1" [dev-dependencies] @@ -45,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", rev = "53e5a89851fcfeeae404b9ab0ca461f149a0ff2b", 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"