Skip to content

Commit dca26fa

Browse files
maltesanderclaude
andcommitted
feat: make SQL_ATTR_CURRENT_CATALOG readable and applied, and export SQL_PARAM_*
The spec makes SQL_ATTR_CURRENT_CATALOG and SQL_DATABASE_NAME one value under two names, but the attribute was handle-local and write-only: SQLGetConnectAttr answered "" while SQLGetInfo answered the real catalog, and setting it returned SQL_SUCCESS while switching nothing. Reported from the Trino driver, where the derivation added in b2f79b7 was correct in principle and inert in practice. Two defaulted Backend methods close it. current_catalog reports the session's catalog, and both readers now consult the same two sources in the same order — what the application set, else what the session is using. set_current_catalog applies a change, and SQLSetConnectAttr stores the value only once it succeeds; the default reports HYC00, since storing a catalog the session never switched to tells an application its unqualified names resolve somewhere they do not. A value set before connecting is applied at connect, alongside autocommit and isolation. SQL_PARAM_SUCCESS and its four siblings move from pub(crate) in ffi::params to public constants in types. odbc-sys lacks them, so a driver asserting what its own executions wrote into SQL_ATTR_PARAM_STATUS_PTR had to declare local copies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0c38e91 commit dca26fa

9 files changed

Lines changed: 201 additions & 23 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,24 @@ Everything a driver has to change for the catalog rework, in one place.
5656

5757
### Added
5858

59+
- `Backend::current_catalog` and `Backend::set_current_catalog`, both defaulted,
60+
make `SQL_ATTR_CURRENT_CATALOG` more than a handle-local string.
61+
`SQLGetConnectAttr` and `SQLGetInfo(SQL_DATABASE_NAME)` — one value under two
62+
names, per the spec — now read the same two sources in the same order: what
63+
the application set, else what the session is actually using. Without the
64+
second source the attribute was write-only, answering `""` while the info type
65+
answered the real catalog. `SQLSetConnectAttr` asks the backend to switch and
66+
stores the value only if that succeeds; the default reports `HYC00`, because
67+
storing a catalog the session never switched to tells an application its
68+
unqualified names resolve somewhere they do not. A value set before connecting
69+
is applied at connect, like autocommit and isolation.
70+
71+
- `SQL_PARAM_SUCCESS`, `SQL_PARAM_ERROR`, `SQL_PARAM_SUCCESS_WITH_INFO`,
72+
`SQL_PARAM_UNUSED` and `SQL_PARAM_DIAG_UNAVAILABLE` are public in
73+
`types`. `odbc-sys` does not define them, and a driver asserting what its own
74+
executions wrote into `SQL_ATTR_PARAM_STATUS_PTR` needs to name them rather
75+
than declare local copies.
76+
5977
- `conformance::info_group_inconsistencies` checks the `SQLGetInfo` groups whose
6078
members constrain each other, and returns one message per violation. Core
6179
cannot police a backend's `get_info` at runtime — that method runs first and

‎src/backend.rs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,42 @@ pub trait Backend: Sized + Send + Sync + 'static {
179179
info_type: crate::types::InfoType,
180180
) -> Result<InfoValue, Self::Error>;
181181

182+
/// The catalog the session is currently using, if the data source has a
183+
/// current catalog at all.
184+
///
185+
/// Read by `SQLGetConnectAttr(SQL_ATTR_CURRENT_CATALOG)` and, because the
186+
/// spec makes them one value, by `SQLGetInfo(SQL_DATABASE_NAME)`. An
187+
/// application's own `SQLSetConnectAttr` value takes precedence, since that
188+
/// is what it asked for; this answers when it has set nothing.
189+
///
190+
/// Defaults to `None`, meaning "this data source has no current catalog to
191+
/// report" — the empty string, which is what both readers then produce.
192+
///
193+
/// A backend that returns `Some` should also implement
194+
/// [`Backend::set_current_catalog`], or an application can read the catalog
195+
/// and not change it.
196+
fn current_catalog(_conn: &Self::Connection) -> Option<Cow<'static, str>> {
197+
None
198+
}
199+
200+
/// Switch the session to `catalog`.
201+
///
202+
/// Called by `SQLSetConnectAttr(SQL_ATTR_CURRENT_CATALOG)`. The spec's
203+
/// example is a driver sending `USE database`; for a single-tier driver it
204+
/// may be changing a directory.
205+
///
206+
/// The default reports `HYC00`, which is the honest answer for a backend
207+
/// that cannot switch catalogs: storing the value and returning
208+
/// `SQL_SUCCESS` would tell an application its unqualified names now
209+
/// resolve somewhere they do not. Core stores the value only once this
210+
/// returns `Ok`.
211+
fn set_current_catalog(_conn: &Self::Connection, _catalog: &str) -> Result<(), Self::Error> {
212+
Err(OdbcError::NotImplemented {
213+
feature: "SQL_ATTR_CURRENT_CATALOG".into(),
214+
}
215+
.into())
216+
}
217+
182218
/// Return driver-level info that does not require an active connection.
183219
///
184220
/// The Windows Driver Manager calls `SQLGetInfoW` for types like

‎src/ffi/connect_attr.rs‎

Lines changed: 91 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! Generic implementations of SQLSetConnectAttrW and SQLGetConnectAttrW.
22
3+
use std::borrow::Cow;
34
use std::ffi::c_void;
45

56
use odbc_sys::ConnectionAttribute;
@@ -44,7 +45,26 @@ pub(crate) fn apply_pending_connect_attrs<B: Backend>(
4445
handle: &mut ConnectionHandle<B>,
4546
) -> Result<(), OdbcError> {
4647
apply_pending_autocommit::<B>(handle)?;
47-
apply_pending_txn_isolation::<B>(handle)
48+
apply_pending_txn_isolation::<B>(handle)?;
49+
apply_pending_current_catalog::<B>(handle)
50+
}
51+
52+
/// Apply a `SQL_ATTR_CURRENT_CATALOG` value that was set before the connection
53+
/// was open. See [`apply_pending_connect_attrs`].
54+
fn apply_pending_current_catalog<B: Backend>(
55+
handle: &mut ConnectionHandle<B>,
56+
) -> Result<(), OdbcError> {
57+
let Some(catalog) = handle
58+
.attr_strings
59+
.get(&ConnectionAttribute::CURRENT_CATALOG.0)
60+
.cloned()
61+
else {
62+
return Ok(());
63+
};
64+
let Some(connection) = handle.connection.as_ref() else {
65+
return Ok(());
66+
};
67+
B::set_current_catalog(connection, &catalog).into_odbc()
4868
}
4969

5070
/// Apply a `SQL_ATTR_AUTOCOMMIT` value that was set before the connection was
@@ -382,7 +402,12 @@ pub unsafe fn sql_set_connect_attr_w<B: Backend>(
382402
Ok(SqlReturn::SUCCESS)
383403
}
384404

385-
// String-valued: SQL_ATTR_CURRENT_CATALOG — decode UTF-16.
405+
// String-valued: SQL_ATTR_CURRENT_CATALOG — decode UTF-16, then
406+
// ask the backend to switch. Storing without switching would
407+
// tell an application its unqualified names now resolve
408+
// somewhere they do not; a backend that cannot switch reports
409+
// HYC00 from the default `set_current_catalog`, and nothing is
410+
// stored.
386411
_ if attr == ConnectionAttribute::CURRENT_CATALOG => {
387412
if value_ptr.is_null() {
388413
conn.attr_strings.remove(&attribute);
@@ -400,6 +425,14 @@ pub unsafe fn sql_set_connect_attr_w<B: Backend>(
400425
string_length / 2
401426
};
402427
let s = utf16_to_string(value_ptr as *const u16, len_code_units)?;
428+
// Applied here when connected; a value set before the
429+
// connection exists is stored and applied by
430+
// `apply_pending_current_catalog` at connect, since the
431+
// spec lists this attribute as settable either side of
432+
// one.
433+
if let Some(connection) = conn.connection.as_ref() {
434+
B::set_current_catalog(connection, &s).into_odbc()?;
435+
}
403436
conn.attr_strings.insert(attribute, s);
404437
}
405438
Ok(SqlReturn::SUCCESS)
@@ -652,12 +685,22 @@ pub unsafe fn sql_get_connect_attr_w<B: Backend>(
652685
Ok(SqlReturn::SUCCESS)
653686
}
654687

655-
// SQL_ATTR_CURRENT_CATALOG: return stored string or empty.
688+
// SQL_ATTR_CURRENT_CATALOG: what the application set, else what
689+
// the session is actually using. Without the second half this
690+
// attribute is write-only — it answers "" while
691+
// `SQLGetInfo(SQL_DATABASE_NAME)`, which the spec makes the same
692+
// value, answers the real catalog.
656693
_ if attr == ConnectionAttribute::CURRENT_CATALOG => {
657694
let s = conn
658695
.attr_strings
659696
.get(&attribute)
660697
.cloned()
698+
.or_else(|| {
699+
conn.connection
700+
.as_ref()
701+
.and_then(|c| B::current_catalog(c))
702+
.map(Cow::into_owned)
703+
})
661704
.unwrap_or_default();
662705
// write_utf16 takes buf_len and len_ptr as i16 (SQLSMALLINT),
663706
// but GetConnectAttrW uses i32 (SQLINTEGER). Validate the
@@ -702,7 +745,7 @@ mod tests {
702745
use super::*;
703746
use crate::ffi::handle::{sql_alloc_handle, sql_free_handle};
704747
use crate::test_utils::{
705-
MockBackend, MockConnection, MockIsolationBackend, MockIsolationConnection,
748+
MockAltBackend, MockBackend, MockConnection, MockIsolationBackend, MockIsolationConnection,
706749
MockUnappliedIsolationBackend, with_handle,
707750
};
708751
use odbc_sys::HandleType;
@@ -932,6 +975,50 @@ mod tests {
932975
}
933976
}
934977

978+
/// `SQL_ATTR_CURRENT_CATALOG` and `SQL_DATABASE_NAME` are one value under
979+
/// two names, so they must read the same two sources in the same order:
980+
/// what the application set, else what the session is actually using. With
981+
/// only the first, the attribute is write-only — it answers `""` while the
982+
/// info type answers the real catalog.
983+
#[test]
984+
fn current_catalog_falls_back_to_the_session_catalog() {
985+
unsafe {
986+
let (env, conn) = alloc_env_conn_for::<MockAltBackend>();
987+
assert_eq!(driver_connect::<MockAltBackend>(conn), SqlReturn::SUCCESS);
988+
989+
let mut buf = [0u16; 64];
990+
let mut len: i32 = 0;
991+
assert_eq!(
992+
sql_get_connect_attr_w::<MockAltBackend>(
993+
conn,
994+
ConnectionAttribute::CURRENT_CATALOG.0,
995+
buf.as_mut_ptr().cast(),
996+
(buf.len() * 2) as i32,
997+
&mut len,
998+
),
999+
SqlReturn::SUCCESS
1000+
);
1001+
let attr = String::from_utf16_lossy(&buf[..(len / 2) as usize]);
1002+
assert_eq!(
1003+
attr, "alt_catalog",
1004+
"the attribute must report the session catalog when the \
1005+
application has set none"
1006+
);
1007+
1008+
// The same value under its other name.
1009+
let (_, info) = crate::conformance::observe_string_value::<MockAltBackend>(
1010+
conn,
1011+
crate::types::SQL_DATABASE_NAME,
1012+
);
1013+
assert_eq!(
1014+
info, attr,
1015+
"SQL_DATABASE_NAME and SQL_ATTR_CURRENT_CATALOG disagree"
1016+
);
1017+
1018+
cleanup_for::<MockAltBackend>(env, conn);
1019+
}
1020+
}
1021+
9351022
/// `SQL_ATTR_ASYNC_ENABLE` and `SQL_ATTR_ODBC_CURSORS` are the two
9361023
/// connection attributes the spec declares `SQLULEN` — "A SQLULEN value
9371024
/// that specifies whether a function called with a statement on the

‎src/ffi/execute.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ use crate::utf16::utf16_to_string;
1818
/// See [`crate::ffi::params::report_params_processed`].
1919
unsafe fn report_param_set<B: Backend>(stmt: &StatementHandle<B>, succeeded: bool) {
2020
let status = if succeeded {
21-
crate::ffi::params::SQL_PARAM_SUCCESS
21+
crate::types::SQL_PARAM_SUCCESS
2222
} else {
23-
crate::ffi::params::SQL_PARAM_ERROR
23+
crate::types::SQL_PARAM_ERROR
2424
};
2525
// SAFETY: the caller's contract, forwarded.
2626
unsafe { crate::ffi::params::report_params_processed(stmt, status) };
@@ -1171,7 +1171,7 @@ mod tests {
11711171
);
11721172
assert_eq!(
11731173
status,
1174-
crate::ffi::params::SQL_PARAM_SUCCESS,
1174+
crate::types::SQL_PARAM_SUCCESS,
11751175
"SQL_ATTR_PARAM_STATUS_PTR was not written"
11761176
);
11771177

@@ -1218,7 +1218,7 @@ mod tests {
12181218
);
12191219

12201220
assert_eq!(processed, 1);
1221-
assert_eq!(status, crate::ffi::params::SQL_PARAM_ERROR);
1221+
assert_eq!(status, crate::types::SQL_PARAM_ERROR);
12221222

12231223
cleanup_env_conn_stmt_for::<MockBackend>(env, conn, stmt);
12241224
}

‎src/ffi/info.rs‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,10 +359,21 @@ pub unsafe fn sql_get_info_w<B: Backend>(
359359
// `SQL_ATTR_CURRENT_CATALOG`, which the spec makes the same value
360360
// as `SQL_DATABASE_NAME`. Read before the dispatch below, which
361361
// borrows the handle's diagnostics mutably.
362+
// What the application set, else what the session is actually
363+
// using — the same two sources, in the same order, that
364+
// `SQLGetConnectAttr(SQL_ATTR_CURRENT_CATALOG)` reads. The spec
365+
// makes these one value, so they must not consult different things.
362366
let current_catalog = handle
363367
.attr_strings
364368
.get(&odbc_sys::ConnectionAttribute::CURRENT_CATALOG.0)
365-
.cloned();
369+
.cloned()
370+
.or_else(|| {
371+
handle
372+
.connection
373+
.as_ref()
374+
.and_then(|c| B::current_catalog(c))
375+
.map(std::borrow::Cow::into_owned)
376+
});
366377
let current_catalog = current_catalog.as_deref();
367378
let info = match handle.connection.as_ref() {
368379
Some(conn) => match info_type_id {

‎src/ffi/params.rs‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ use crate::{
1414
panic::panic_safe,
1515
types::{
1616
ColumnValue, ParamDescriptor, SQL_DATA_AT_EXEC, SQL_DEFAULT_PARAM_SIZE,
17-
SQL_LEN_DATA_AT_EXEC_OFFSET, SQL_NTS, SQL_NULL_DATA, SqlReturn, SqlState, ULen,
18-
c_data_type_from_raw, param_type_from_raw,
17+
SQL_LEN_DATA_AT_EXEC_OFFSET, SQL_NTS, SQL_NULL_DATA, SQL_PARAM_ERROR, SQL_PARAM_SUCCESS,
18+
SqlReturn, SqlState, ULen, c_data_type_from_raw, param_type_from_raw,
1919
},
2020
utf16::utf16_to_string,
2121
};
@@ -855,13 +855,6 @@ pub(crate) unsafe fn collect_params(
855855
/// # Safety
856856
/// Every output binding's `value_ptr` / `str_len_or_ind_ptr` must point to a
857857
/// valid writable buffer, as guaranteed by the `SQLBindParameter` contract.
858-
/// `SQL_PARAM_SUCCESS` — the parameter-status value for a set the data source
859-
/// processed without a diagnostic.
860-
pub(crate) const SQL_PARAM_SUCCESS: u16 = 0;
861-
/// `SQL_PARAM_ERROR` — the parameter-status value for a set whose execution
862-
/// failed.
863-
pub(crate) const SQL_PARAM_ERROR: u16 = 5;
864-
865858
/// Write one processed parameter set through `SQL_ATTR_PARAMS_PROCESSED_PTR`
866859
/// and `status` into the first element of `SQL_ATTR_PARAM_STATUS_PTR`, when the
867860
/// application set either.

‎src/test_utils.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,11 @@ impl Backend for MockBackend {
468468
fn quoted_identifier_case(_conn: &Self::Connection) -> u16 {
469469
crate::types::SQL_IC_SENSITIVE
470470
}
471+
// Switching succeeds; `current_catalog` stays at its `None` default, so the
472+
// getter's application-set branch is what these tests exercise.
473+
fn set_current_catalog(_conn: &Self::Connection, _catalog: &str) -> Result<(), MockError> {
474+
Ok(())
475+
}
471476
// Non-`SQL_TC_NONE`, as `txn_isolation_options` above declares a level.
472477
fn txn_capable(_conn: &Self::Connection) -> u16 {
473478
crate::types::SQL_TC_ALL as u16
@@ -945,6 +950,15 @@ impl Backend for MockAltBackend {
945950
fn quoted_identifier_case(_conn: &Self::Connection) -> u16 {
946951
crate::types::SQL_IC_LOWER
947952
}
953+
// Reports a session catalog, so the getter's fallback branch — and
954+
// `SQL_DATABASE_NAME`, which reads the same two sources — have something to
955+
// find when the application has set nothing.
956+
fn current_catalog(_conn: &Self::Connection) -> Option<Cow<'static, str>> {
957+
Some(Cow::Borrowed("alt_catalog"))
958+
}
959+
fn set_current_catalog(_conn: &Self::Connection, _catalog: &str) -> Result<(), MockError> {
960+
Ok(())
961+
}
948962
fn txn_capable(_conn: &Self::Connection) -> u16 {
949963
crate::types::SQL_TC_DML as u16
950964
}

‎src/types/constants.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,24 @@ pub const SQL_NC_START: u16 = 0x0002;
199199
/// the `ASC` / `DESC` keywords.
200200
pub const SQL_NC_END: u16 = 0x0004;
201201

202+
/// The `SQL_PARAM_*` values a driver writes into the array
203+
/// `SQL_ATTR_PARAM_STATUS_PTR` points at, one per parameter set an execution
204+
/// processed. Absent from `odbc-sys`, and public here because a driver
205+
/// asserting what its own executions wrote needs to name them.
206+
///
207+
/// `SQL_PARAM_SUCCESS` — the set was processed without a diagnostic.
208+
pub const SQL_PARAM_SUCCESS: u16 = 0;
209+
/// `SQL_PARAM_DIAG_UNAVAILABLE` — the set produced a diagnostic the driver
210+
/// cannot attribute to it individually.
211+
pub const SQL_PARAM_DIAG_UNAVAILABLE: u16 = 1;
212+
/// `SQL_PARAM_ERROR` — the set's execution failed.
213+
pub const SQL_PARAM_ERROR: u16 = 5;
214+
/// `SQL_PARAM_SUCCESS_WITH_INFO` — processed, with a diagnostic raised for it.
215+
pub const SQL_PARAM_SUCCESS_WITH_INFO: u16 = 6;
216+
/// `SQL_PARAM_UNUSED` — not processed, because an earlier set's failure ended
217+
/// the execution.
218+
pub const SQL_PARAM_UNUSED: u16 = 7;
219+
202220
/// `SQL_UNSPECIFIED` — it is unspecified whether cursors make visible the
203221
/// changes another cursor made to a result set; they "may make visible none,
204222
/// some, or all such changes". The spec's default for the

‎src/types/mod.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,12 @@ pub use constants::{
8181
SQL_NTS, SQL_NULL_DATA, SQL_NUMERIC_FUNCTIONS, SQL_ODBC_API_CONFORMANCE,
8282
SQL_ODBC_SAG_CLI_CONFORMANCE, SQL_ODBC_SQL_CONFORMANCE, SQL_OIC_CORE,
8383
SQL_OJ_ALL_COMPARISON_OPS, SQL_OJ_FULL, SQL_OJ_INNER, SQL_OJ_LEFT, SQL_OJ_NESTED,
84-
SQL_OJ_NOT_ORDERED, SQL_OJ_RIGHT, SQL_OUTER_JOINS, SQL_PARC_BATCH, SQL_PARC_NO_BATCH,
85-
SQL_PAS_BATCH, SQL_PAS_NO_BATCH, SQL_PAS_NO_SELECT, SQL_PC_NOT_PSEUDO, SQL_PC_PSEUDO,
86-
SQL_PC_UNKNOWN, SQL_POSITION, SQL_PRED_BASIC, SQL_PRED_CHAR, SQL_PRED_NONE, SQL_PROCEDURE_TERM,
87-
SQL_PROCEDURES, SQL_PT_FUNCTION, SQL_PT_PROCEDURE, SQL_PT_UNKNOWN, SQL_QUICK,
88-
SQL_QUOTED_IDENTIFIER_CASE, SQL_REFRESH, SQL_RESTRICT, SQL_ROW_UPDATES, SQL_ROWVER,
84+
SQL_OJ_NOT_ORDERED, SQL_OJ_RIGHT, SQL_OUTER_JOINS, SQL_PARAM_DIAG_UNAVAILABLE, SQL_PARAM_ERROR,
85+
SQL_PARAM_SUCCESS, SQL_PARAM_SUCCESS_WITH_INFO, SQL_PARAM_UNUSED, SQL_PARC_BATCH,
86+
SQL_PARC_NO_BATCH, SQL_PAS_BATCH, SQL_PAS_NO_BATCH, SQL_PAS_NO_SELECT, SQL_PC_NOT_PSEUDO,
87+
SQL_PC_PSEUDO, SQL_PC_UNKNOWN, SQL_POSITION, SQL_PRED_BASIC, SQL_PRED_CHAR, SQL_PRED_NONE,
88+
SQL_PROCEDURE_TERM, SQL_PROCEDURES, SQL_PT_FUNCTION, SQL_PT_PROCEDURE, SQL_PT_UNKNOWN,
89+
SQL_QUICK, SQL_QUOTED_IDENTIFIER_CASE, SQL_REFRESH, SQL_RESTRICT, SQL_ROW_UPDATES, SQL_ROWVER,
8990
SQL_SC_FIPS127_2_TRANSITIONAL, SQL_SC_SQL92_ENTRY, SQL_SC_SQL92_FULL,
9091
SQL_SC_SQL92_INTERMEDIATE, SQL_SCOPE_CURROW, SQL_SCOPE_SESSION, SQL_SCOPE_TRANSACTION,
9192
SQL_SEARCHABLE, SQL_SENSITIVE, SQL_SET_DEFAULT, SQL_SET_NULL, SQL_SO_FORWARD_ONLY,

0 commit comments

Comments
 (0)