fix: a zero-length SQLGetData buffer reports truncation instead of consuming the column

write_wchar/write_char/write_binary shared one branch for a null target
and a zero-length buffer, answering SQL_SUCCESS with nothing written —
and sql_get_data marks a column done on any non-SUCCESS_WITH_INFO
return, so the standard length-probe idiom (call with BufferLength 0 to
read the indicator, then call again with a real buffer) got SQL_NO_DATA
on the second call and the value was unrecoverable. The two cases are
now split the way write_utf16 already ruled: null target stays a pure
length query (SUCCESS), a non-null target with no room is total
truncation (SUCCESS_WITH_INFO + 01004) and the cursor stays resumable.
The same split reaches SQLFetch's bound columns through the shared
writers, so a BufferLength 0 binding now reports 01004 too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: adwk67

CHANGELOG.md

@@ -3085,4 +3085,30 @@ call `SQLCloseCursor` or `SQLFreeStmt(SQL_CLOSE)` first, as it already must for
   function returns changed; the previously-unguarded path now returns one
   (`HY000`) instead of aborting.
 
+- **`SQLGetData` with a zero-length buffer no longer consumes the column.**
+  `write_wchar`, `write_char` and `write_binary` shared one branch for a null
+  `target_value_ptr` (a pure length query) and a non-null one with
+  `buffer_length` 0 (the standard "how large a buffer do I need" probe): both
+  returned plain `SQL_SUCCESS`. `SQLGetData`'s own `cursor.done` is derived
+  from that return value — anything other than `SQL_SUCCESS_WITH_INFO` marks a
+  chunkable column exhausted — so the probe's `SQL_SUCCESS` silently closed the
+  column, and the documented follow-up call with a buffer sized from the
+  reported length got `SQL_NO_DATA` instead of the value. The spec's own step
+  5 already draws this line: "If the data buffer supplied is too small to hold
+  the null-termination character, SQLGetData returns SQL_SUCCESS_WITH_INFO and
+  SQLSTATE 01004" — a zero-length buffer is always too small to hold it, so
+  it is the same case as any other partial write, not a length query. The
+  three writers now split the same way `write_utf16` already does elsewhere in
+  this crate: null target stays `SQL_SUCCESS`, non-null target with no room
+  (`buffer_length <= 0`, or `< 2` for `SQL_C_WCHAR`'s two-byte terminator)
+  becomes `SQL_SUCCESS_WITH_INFO` with `01004` and the cursor left resumable.
+
+  **For driver authors:** the same three writers also serve `SQLFetch`'s
+  bound-column path (`SQLBindCol` with `BufferLength` 0), which now reports
+  `SQL_SUCCESS_WITH_INFO`/`01004` for that row instead of silently discarding
+  the value — the same shared-branch bug, on a call shape no test in this
+  crate previously exercised. A `SQLGetData` call with a non-zero
+  `BufferLength`, or any bound column with a real `BufferLength`, is
+  unaffected.
+
 [Unreleased]: https://github.com/stackabletech/stackable-odbc-core/commits/HEAD

src/column_value.rs

@@ -895,15 +895,37 @@ unsafe fn write_wchar(
         unsafe { std::ptr::write_unaligned(len_ind_ptr, total_bytes) };
     }
 
-    if target_ptr.is_null() || buf_len <= 0 {
+    // A null target is not something SQLGetData's own spec sanctions — its
+    // Arguments section is explicit that "TargetValuePtr cannot be NULL."
+    // The case that actually reaches this branch comes from this function's
+    // *other* caller: `sql_fetch`'s bound-column loop (`ffi/fetch.rs`)
+    // legitimately passes a null data pointer when `SQL_DESC_DATA_PTR` is
+    // null but `SQL_DESC_INDICATOR_PTR` is not — the indicator-only binding
+    // the spec allows ("An application can unbind the data buffer for a
+    // column but still have a length/indicator buffer bound for the
+    // column"), which `collect_bindings` deliberately keeps and
+    // `fetch_writes_the_indicator_of_an_indicator_only_binding` pins. That
+    // caller still wants the length written to `len_ind_ptr` above, with
+    // nothing written to a buffer that does not exist, so this returns
+    // SUCCESS rather than treating the null pointer as an error this deep in
+    // the call stack; `SQLGetData`'s own `target_value_ptr` null case is
+    // `HY009`, deliberately left unchecked at the FFI boundary (see
+    // `sql_get_data`'s doc comment) rather than enforced here.
+    if target_ptr.is_null() {
         return Ok((SqlReturn::SUCCESS, 0));
     }
 
-    // The null terminator is one UTF-16 code unit, so a buffer of fewer than
-    // two bytes cannot hold it. Writing one anyway would overrun the caller's
-    // buffer. Spec: "If the data buffer supplied is too small to hold the
-    // null-termination character, SQLGetData returns SQL_SUCCESS_WITH_INFO
-    // and SQLSTATE 01004."
+    // A non-null target with fewer than two bytes of room — including
+    // exactly zero, the standard "how big a buffer do I need" probe — cannot
+    // hold even the one-UTF-16-code-unit null terminator. That is total
+    // truncation, not a length query: the application supplied somewhere to
+    // write and nothing was written there. Spec: "If the data buffer
+    // supplied is too small to hold the null-termination character,
+    // SQLGetData returns SQL_SUCCESS_WITH_INFO and SQLSTATE 01004." Reporting
+    // plain SUCCESS here (as a shared branch with the null-target case above
+    // used to) made SQLGetData indistinguishable from "this column is fully
+    // delivered," which permanently stranded the data behind a `buf_len == 0`
+    // probe: `cursor.done` is derived from this return value.
     if buf_len < 2 {
         return Ok((SqlReturn::SUCCESS_WITH_INFO, 0));
     }
@@ -957,10 +979,21 @@ unsafe fn write_char(
         unsafe { std::ptr::write_unaligned(len_ind_ptr, total_bytes) };
     }
 
-    if target_ptr.is_null() || buf_len <= 0 {
+    // A null target here is the bound-column caller's indicator-only
+    // binding, not something SQLGetData's own spec permits — see
+    // `write_wchar`'s full reasoning.
+    if target_ptr.is_null() {
         return Ok((SqlReturn::SUCCESS, 0));
     }
 
+    // A non-null target with no room in it — including exactly zero, the
+    // standard length-probe — cannot hold even the one-byte null terminator,
+    // which is total truncation (SUCCESS_WITH_INFO / 01004), not a length
+    // query. See `write_wchar`'s identical split.
+    if buf_len <= 0 {
+        return Ok((SqlReturn::SUCCESS_WITH_INFO, 0));
+    }
+
     let out_ptr = target_ptr.cast::<u8>();
     let capacity = (buf_len as usize).saturating_sub(1); // reserve one for null terminator
     let copy_count = bytes.len().min(capacity);
@@ -1001,10 +1034,23 @@ unsafe fn write_binary(
         unsafe { std::ptr::write_unaligned(len_ind_ptr, total_bytes) };
     }
 
-    if target_ptr.is_null() || buf_len <= 0 {
+    // A null target here is the bound-column caller's indicator-only
+    // binding, not something SQLGetData's own spec permits — see
+    // `write_wchar`'s full reasoning.
+    if target_ptr.is_null() {
         return Ok((SqlReturn::SUCCESS, 0));
     }
 
+    // A non-null target with no room in it — including exactly zero, the
+    // standard length-probe — cannot hold any of the data, which is total
+    // truncation (SUCCESS_WITH_INFO / 01004), not a length query. Binary
+    // reserves no terminator, so unlike the two character writers there is no
+    // extra "room for one more byte" boundary; `buf_len <= 0` is the whole
+    // condition. See `write_wchar`'s identical split.
+    if buf_len <= 0 {
+        return Ok((SqlReturn::SUCCESS_WITH_INFO, 0));
+    }
+
     let out_ptr = target_ptr.cast::<u8>();
     let copy_count = data.len().min(buf_len as usize);
 
@@ -1888,7 +1934,10 @@ mod tests {
 
     #[test]
     fn wchar_zero_length_buffer_reports_size_and_writes_nothing() {
-        // buf_len == 0 is a length query: report the byte count, write nothing.
+        // A non-null target with buf_len == 0 has no room for even the null
+        // terminator, which is total truncation (SUCCESS_WITH_INFO / 01004),
+        // not a length query. Only a null target is a length query
+        // (SUCCESS) — see `write_utf16`'s identical split.
         let mut buf = [0xAAu8; 4];
         let mut ind: isize = 0;
         let ret = unsafe {
@@ -1900,11 +1949,74 @@ mod tests {
                 &mut ind,
             )
         };
-        assert_eq!(ret.unwrap(), SqlReturn::SUCCESS);
+        assert_eq!(ret.unwrap(), SqlReturn::SUCCESS_WITH_INFO);
         assert_eq!(ind, 10); // 5 chars * 2 bytes, still reported
         assert_eq!(buf, [0xAA; 4], "wrote into a zero-length buffer");
     }
 
+    #[test]
+    fn wchar_null_target_with_zero_length_is_a_pure_length_query() {
+        // A null target pointer stays SUCCESS regardless of buf_len. Not
+        // something SQLGetData's own spec sanctions directly — its Arguments
+        // section says "TargetValuePtr cannot be NULL" — but this writer is
+        // shared with `sql_fetch`'s bound-column loop, where a null
+        // `SQL_DESC_DATA_PTR` paired with a live indicator pointer is the
+        // spec-legal indicator-only binding (see `write_wchar`'s doc
+        // comment on this branch for the full reasoning).
+        let mut ind: isize = 0;
+        let ret = unsafe {
+            write_column_value(
+                &ColumnValue::String("hello".into()),
+                CDataType::WChar,
+                std::ptr::null_mut(),
+                0,
+                &mut ind,
+            )
+        };
+        assert_eq!(ret.unwrap(), SqlReturn::SUCCESS);
+        assert_eq!(ind, 10);
+    }
+
+    #[test]
+    fn char_zero_length_buffer_reports_size_and_writes_nothing() {
+        // The write_char sibling of the wchar case above.
+        let mut buf = [0xAAu8; 4];
+        let mut ind: isize = 0;
+        let ret = unsafe {
+            write_column_value(
+                &ColumnValue::String("hello".into()),
+                CDataType::Char,
+                buf.as_mut_ptr() as *mut c_void,
+                0,
+                &mut ind,
+            )
+        };
+        assert_eq!(ret.unwrap(), SqlReturn::SUCCESS_WITH_INFO);
+        assert_eq!(ind, 5);
+        assert_eq!(buf, [0xAA; 4], "wrote into a zero-length buffer");
+    }
+
+    #[test]
+    fn binary_zero_length_buffer_reports_size_and_writes_nothing() {
+        // The write_binary sibling of the wchar case above. Binary has no
+        // null terminator, so the "no room to make progress" condition is
+        // simply buf_len <= 0 rather than needing 2 bytes.
+        let mut buf = [0xAAu8; 4];
+        let mut ind: isize = 0;
+        let ret = unsafe {
+            write_column_value(
+                &ColumnValue::Bytes(vec![0xDE, 0xAD, 0xBE, 0xEF]),
+                CDataType::Binary,
+                buf.as_mut_ptr() as *mut c_void,
+                0,
+                &mut ind,
+            )
+        };
+        assert_eq!(ret.unwrap(), SqlReturn::SUCCESS_WITH_INFO);
+        assert_eq!(ind, 4);
+        assert_eq!(buf, [0xAA; 4], "wrote into a zero-length buffer");
+    }
+
     #[test]
     fn wchar_buffer_holding_only_the_null_terminator_is_written() {
         // Two bytes is exactly one UTF-16 code unit: room for the terminator