From 8d32299347fb365d7835f5ae9c10f5635c9c0e06 Mon Sep 17 00:00:00 2001
From: Linwei Zhang <lewiszlw520@gmail.com>
Date: Mon, 22 Jun 2026 13:48:52 +0800
Subject: [PATCH 1/2] refactor(mdb): use safe conn.execute() for query instead
 of unsafe exec_direct

Replace the manual preallocate + unsafe exec_direct + unsafe CursorImpl::new
with odbc-api's safe conn.execute(&sql, (), None), matching the infer_schema
and DM connection patterns. The mdbtools dummy-column bind workaround remains
but is now threaded through cursor.as_stmt_ref().
---
 remote-table/src/connection/mdb/mod.rs | 45 +++++++++-----------------
 1 file changed, 15 insertions(+), 30 deletions(-)

diff --git a/remote-table/src/connection/mdb/mod.rs b/remote-table/src/connection/mdb/mod.rs
index 9c6011a..5454a6e 100644
--- a/remote-table/src/connection/mdb/mod.rs
+++ b/remote-table/src/connection/mdb/mod.rs
@@ -19,7 +19,7 @@ use datafusion_physical_plan::stream::RecordBatchStreamAdapter;
 use futures::lock::Mutex;
 use log::debug;
 use odbc_api::Environment;
-use odbc_api::handles::{SqlResult, SqlText, Statement, StatementImpl};
+use odbc_api::handles::{AsStatementRef, SqlResult, SqlText, Statement, StatementImpl};
 use odbc_api::{Cursor, CursorImpl};
 use std::collections::HashMap;
 use std::path::PathBuf;
@@ -236,40 +236,28 @@ impl Connection for MdbConnection {
             let handle = Handle::current();
             let conn = handle.block_on(async { conn.lock().await });
 
-            // mdbtools does not support SQL_ATTR_PARAMSET_SIZE, so odbc-api's safe
-            // `conn.execute()` fails with HY092. Use SQLExecDirect (like the
-            // mdbtools C unit test does) followed by SQLBindCol + SQLFetch.
-            let pre = conn.preallocate().map_err(|e| {
-                DataFusionError::Execution(format!(
-                    "Failed to preallocate statement for mdb query: {e:?}"
-                ))
-            })?;
-            let mut stmt = pre.into_handle();
-            let sql_text = SqlText::new(&sql);
-            // SAFETY: `sql` is owned by this closure and outlives `stmt`.
-            // `exec_direct` is unsafe (may dereference bound parameters; we have none).
-            if unsafe { stmt.exec_direct(&sql_text) }.is_err() {
-                return Err(DataFusionError::Execution(format!(
-                    "Failed to execute query on mdb: {sql}"
-                )));
-            }
+            let mut cursor = conn.execute(&sql, (), None)
+                .map_err(|e| {
+                    DataFusionError::Execution(format!(
+                        "Failed to execute query on mdb: {e:?}, sql: {sql}"
+                    ))
+                })?
+                .ok_or_else(|| {
+                    DataFusionError::Execution(format!(
+                        "No result set returned for mdb query: {sql}"
+                    ))
+                })?;
+
             // mdbtools 1.0.x requires SQLBindCol before SQLFetch — without it,
             // SQLFetch hangs indefinitely. odbc-api's Cursor::next_row() calls
-            // SQLFetch without binding columns, so we bypass CursorImpl entirely
-            // and use the Statement trait's bind_col() + fetch() directly.
-            // Each column is bound to a single-row buffer; each fetch() call
-            // writes one row into the bound buffers.
-
-            // mdbtools' SQLFetch requires at least one column to be bound with
-            // SQLBindCol before the first SQLFetch call, otherwise it hangs.
-            // However, with all columns bound, SQLFetch never returns SQL_NO_DATA.
+            // SQLFetch without binding columns.
             //
             // Workaround: bind a single dummy column (column 1) to allow
             // SQLFetch to proceed, then use the row-by-row Cursor::next_row()
             // path which calls SQLGetData for each cell. SQLGetData after
             // SQLBindCol works because the dummy buffer is ignored.
             let mut dummy = odbc_api::Nullable::<i32>::null();
-            match unsafe { stmt.bind_col(1, &mut dummy) } {
+            match unsafe { cursor.as_stmt_ref().bind_col(1, &mut dummy) } {
                 SqlResult::Success(()) | SqlResult::SuccessWithInfo(()) => {}
                 SqlResult::Error { function } => {
                     return Err(DataFusionError::Execution(format!(
@@ -282,9 +270,6 @@ impl Connection for MdbConnection {
                     )));
                 }
             }
-
-            // SAFETY: stmt is in cursor state after exec_direct.
-            let mut cursor: CursorImpl<StatementImpl> = unsafe { CursorImpl::new(stmt) };
             let mut exhausted = false;
 
             loop {

From e7f236960abcbd652c5ac5336459a9cd8e45f011 Mon Sep 17 00:00:00 2001
From: Linwei Zhang <lewiszlw520@gmail.com>
Date: Mon, 22 Jun 2026 13:53:12 +0800
Subject: [PATCH 2/2] chore: cargo fmt

---
 remote-table/src/connection/mdb/mod.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/remote-table/src/connection/mdb/mod.rs b/remote-table/src/connection/mdb/mod.rs
index 5454a6e..52f7ee7 100644
--- a/remote-table/src/connection/mdb/mod.rs
+++ b/remote-table/src/connection/mdb/mod.rs
@@ -236,7 +236,8 @@ impl Connection for MdbConnection {
             let handle = Handle::current();
             let conn = handle.block_on(async { conn.lock().await });
 
-            let mut cursor = conn.execute(&sql, (), None)
+            let mut cursor = conn
+                .execute(&sql, (), None)
                 .map_err(|e| {
                     DataFusionError::Execution(format!(
                         "Failed to execute query on mdb: {e:?}, sql: {sql}"