Use indicator buffer to check for trunactions

Changelog.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.54.0
+
+* Changed checking of truncated values after fetch. The new method checks the indicator buffer, rather than relying on diagnostics. This means `Error::TooManyDiagnostics` can no longer occurr.
+
 ## 0.53.0
 
 * Better error messages if checking for truncation fails due to too many diagnostics

odbc-api/src/buffers/columnar.rs

@@ -104,8 +104,10 @@ where
         Ok(())
     }
 
-    fn has_truncated_values(&mut self) -> bool {
-        self.columns.iter().any(|col_buffer| col_buffer.1.has_truncated_values(*self.num_rows))
+    fn has_truncated_values(&self) -> bool {
+        self.columns
+            .iter()
+            .any(|col_buffer| col_buffer.1.has_truncated_values(*self.num_rows))
     }
 }
 

odbc-api/src/cursor.rs

@@ -336,7 +336,7 @@ pub unsafe trait RowSetBuffer {
     unsafe fn bind_colmuns_to_cursor(&mut self, cursor: StatementRef<'_>) -> Result<(), Error>;
 
     /// Check if the buffer contains any truncated values for variadic sized columns.
-    fn has_truncated_values(&mut self) -> bool;
+    fn has_truncated_values(&self) -> bool;
 }
 
 unsafe impl<T: RowSetBuffer> RowSetBuffer for &mut T {
@@ -356,8 +356,8 @@ unsafe impl<T: RowSetBuffer> RowSetBuffer for &mut T {
         (*self).bind_colmuns_to_cursor(cursor)
     }
 
-    fn has_truncated_values(&mut self) -> bool {
-        (*self).has_truncated_values()
+    fn has_truncated_values(&self) -> bool {
+        (**self).has_truncated_values()
     }
 }
 
@@ -414,7 +414,10 @@ where
     ///     }
     /// }
     /// ```
-    pub fn fetch(&mut self) -> Result<Option<&B>, Error> {
+    pub fn fetch(&mut self) -> Result<Option<&B>, Error>
+    where
+        B: RowSetBuffer,
+    {
         self.fetch_with_truncation_check(false)
     }
 
@@ -446,11 +449,15 @@ where
     pub fn fetch_with_truncation_check(
         &mut self,
         error_for_truncation: bool,
-    ) -> Result<Option<&B>, Error> {
+    ) -> Result<Option<&B>, Error>
+    where
+        B: RowSetBuffer,
+    {
         let mut stmt = self.cursor.as_stmt_ref();
         unsafe {
             let result = stmt.fetch();
-            let has_row = error_handling_for_fetch(result, stmt, error_for_truncation)?;
+            let has_row =
+                error_handling_for_fetch(result, stmt, &self.buffer, error_for_truncation)?;
             Ok(has_row.then_some(&self.buffer))
         }
     }
@@ -571,7 +578,10 @@ where
     ///
     /// `None` if the result set is empty and all row sets have been extracted. `Some` with a
     /// reference to the internal buffer otherwise.
-    pub async fn fetch(&mut self, sleep: impl Sleep) -> Result<Option<&B>, Error> {
+    pub async fn fetch(&mut self, sleep: impl Sleep) -> Result<Option<&B>, Error>
+    where
+        B: RowSetBuffer,
+    {
         self.fetch_with_truncation_check(false, sleep).await
     }
 
@@ -589,13 +599,14 @@ where
         &mut self,
         error_for_truncation: bool,
         mut sleep: impl Sleep,
-    ) -> Result<Option<&B>, Error> {
+    ) -> Result<Option<&B>, Error>
+    where
+        B: RowSetBuffer,
+    {
         let mut stmt = self.cursor.as_stmt_ref();
-        unsafe {
-            let result = wait_for(|| stmt.fetch(), &mut sleep).await;
-            let has_row = error_handling_for_fetch(result, stmt, error_for_truncation)?;
-            Ok(has_row.then_some(&self.buffer))
-        }
+        let result = unsafe { wait_for(|| stmt.fetch(), &mut sleep).await };
+        let has_row = error_handling_for_fetch(result, stmt, &self.buffer, error_for_truncation)?;
+        Ok(has_row.then_some(&self.buffer))
     }
 }
 
@@ -630,23 +641,31 @@ unsafe fn bind_row_set_buffer_to_statement(
 fn error_handling_for_fetch(
     result: SqlResult<()>,
     mut stmt: StatementRef,
+    buffer: &impl RowSetBuffer,
     error_for_truncation: bool,
 ) -> Result<bool, Error> {
-    let has_row = result
-        .on_success(|| true)
-        .into_result_with(&stmt.as_stmt_ref(), error_for_truncation, Some(false), None)
-        // Oracles ODBC driver does not support 64Bit integers. Furthermore, it does not
-        // tell the it to the user than binding parameters, but rather now then we fetch
-        // results. The error code retruned is `HY004` rather then `HY003` which should
-        // be used to indicate invalid buffer types.
-        .provide_context_for_diagnostic(|record, function| {
-            if record.state == State::INVALID_SQL_DATA_TYPE {
-                Error::OracleOdbcDriverDoesNotSupport64Bit(record)
-            } else {
-                Error::Diagnostics { record, function }
-            }
-        })?;
-    Ok(has_row)
+    if error_for_truncation
+        && result == SqlResult::SuccessWithInfo(())
+        && buffer.has_truncated_values()
+    {
+        Err(Error::TooLargeValueForBuffer)
+    } else {
+        let has_row = result
+            .on_success(|| true)
+            .into_result_with(&stmt.as_stmt_ref(), Some(false), None)
+            // Oracles ODBC driver does not support 64Bit integers. Furthermore, it does not
+            // tell the it to the user than binding parameters, but rather now then we fetch
+            // results. The error code retruned is `HY004` rather then `HY003` which should
+            // be used to indicate invalid buffer types.
+            .provide_context_for_diagnostic(|record, function| {
+                if record.state == State::INVALID_SQL_DATA_TYPE {
+                    Error::OracleOdbcDriverDoesNotSupport64Bit(record)
+                } else {
+                    Error::Diagnostics { record, function }
+                }
+            })?;
+        Ok(has_row)
+    }
 }
 
 impl<C, B> Drop for BlockCursorPolling<C, B>

odbc-api/src/error.rs

@@ -2,7 +2,7 @@ use std::io;
 
 use thiserror::Error as ThisError;
 
-use crate::handles::{log_diagnostics, Diagnostics, Record as DiagnosticRecord, SqlResult, State};
+use crate::handles::{log_diagnostics, Diagnostics, Record as DiagnosticRecord, SqlResult};
 
 /// Error indicating a failed allocation for a column buffer
 #[derive(Debug)]
@@ -116,15 +116,6 @@ pub enum Error {
         num_elements: usize,
         element_size: usize,
     },
-    #[error(
-        "The number of diagnostic records returned by ODBC seems to exceed `32,767` This means not
-        all (diagnostic) records could be inspected. Sadly this prevents checking for truncated
-        values and puts you at risk of silently loosing data. Usually this many warnings are only
-        generated, if one or more warnings per row is generated, and then there are many rows. Maybe
-        try fewer rows, or fix the cause of some of these warnings/errors? One of these diagnostic
-        records contains:\n{record}."
-    )]
-    TooManyDiagnostics { record: DiagnosticRecord },
     #[error(
         "A value (at least one) is too large to be written into the allocated buffer without
         truncation."
@@ -169,7 +160,7 @@ impl SqlResult<()> {
     /// `Ok(true)`.
     pub fn into_result_bool(self, handle: &impl Diagnostics) -> Result<bool, Error> {
         self.on_success(|| true)
-            .into_result_with(handle, false, Some(false), None)
+            .into_result_with(handle, Some(false), None)
     }
 }
 
@@ -179,16 +170,13 @@ impl<T> SqlResult<T> {
     /// [`Self::Success`] and [`Self::SuccessWithInfo`] are mapped to Ok. In case of
     /// [`Self::SuccessWithInfo`] any diagnostics are logged. [`Self::Error`] is mapped to error.
     pub fn into_result(self, handle: &impl Diagnostics) -> Result<T, Error> {
-        let error_for_truncation = false;
-        self.into_result_with(handle, error_for_truncation, None, None)
+        self.into_result_with(handle, None, None)
     }
 
     /// Like [`Self::into_result`], but [`SqlResult::NoData`] is mapped to `None`, and any success
     /// is mapped to `Some`.
     pub fn into_result_option(self, handle: &impl Diagnostics) -> Result<Option<T>, Error> {
-        let error_for_truncation = false;
-        self.map(Some)
-            .into_result_with(handle, error_for_truncation, Some(None), None)
+        self.map(Some).into_result_with(handle, Some(None), None)
     }
 
     /// Most flexible way of converting an `SqlResult` to an idiomatic `Result`.
@@ -209,7 +197,6 @@ impl<T> SqlResult<T> {
     pub fn into_result_with(
         self,
         handle: &impl Diagnostics,
-        error_for_truncation: bool,
         no_data: Option<T>,
         need_data: Option<T>,
     ) -> Result<T, Error> {
@@ -219,14 +206,6 @@ impl<T> SqlResult<T> {
             // The function has been executed successfully. There have been warnings. Holds result.
             SqlResult::SuccessWithInfo(value) => {
                 log_diagnostics(handle);
-
-                // This is only relevant then bulk fetching values into a buffer. As such this check
-                // would perform unnecessary work in most cases. When bulk fetching it should be up
-                // for the application to decide wether this is considered an error or not.
-                if error_for_truncation {
-                    check_for_truncation(handle)?;
-                }
-
                 Ok(value)
             }
             SqlResult::Error { function } => {
@@ -254,21 +233,3 @@ impl<T> SqlResult<T> {
         }
     }
 }
-
-fn check_for_truncation(handle: &impl Diagnostics) -> Result<(), Error> {
-    let mut empty = [];
-    let mut rec_number = 1;
-    while let Some(result) = handle.diagnostic_record(1, &mut empty) {
-        if result.state == State::STRING_DATA_RIGHT_TRUNCATION {
-            return Err(Error::TooLargeValueForBuffer);
-        } else if rec_number == i16::MAX {
-            // Many diagnostic records may be produced with a single call. Especially in case of
-            // bulk fetching, or inserting. Sadly this could mask truncation errors.
-            let mut record = DiagnosticRecord::with_capacity(512);
-            record.fill_from(handle, i16::MAX);
-            return Err(Error::TooManyDiagnostics { record });
-        }
-        rec_number += 1;
-    }
-    Ok(())
-}

odbc-api/src/execute.rs

@@ -102,10 +102,9 @@ where
 
     // If delayed parameters (e.g. input streams) are bound we might need to put data in order to
     // execute.
-    let need_data =
-        result
-            .on_success(|| false)
-            .into_result_with(&stmt, false, Some(false), Some(true))?;
+    let need_data = result
+        .on_success(|| false)
+        .into_result_with(&stmt, Some(false), Some(true))?;
 
     if need_data {
         // Check if any delayed parameters have been bound which stream data to the database at
@@ -155,10 +154,9 @@ where
 
     // If delayed parameters (e.g. input streams) are bound we might need to put data in order to
     // execute.
-    let need_data =
-        result
-            .on_success(|| false)
-            .into_result_with(&stmt, false, Some(false), Some(true))?;
+    let need_data = result
+        .on_success(|| false)
+        .into_result_with(&stmt, Some(false), Some(true))?;
 
     if need_data {
         // Check if any delayed parameters have been bound which stream data to the database at

odbc-api/tests/common.rs

@@ -209,7 +209,7 @@ where
         Ok(())
     }
 
-    fn has_truncated_values(&mut self) -> bool {
+    fn has_truncated_values(&self) -> bool {
         unimplemented!()
     }
 }