Skip to content

Commit

Permalink
fix false positive truncation for varchar
Browse files Browse the repository at this point in the history
  • Loading branch information
pacman82 committed Sep 25, 2023
1 parent c861aa1 commit 97ec441
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 88 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## 1.0.2

* Fix: `Cursor::fetch_with_truncation_check` did erroneously report truncation errors for `NULL` fields, if the query also triggered any ODBC diagnostic. This has been fixed in the previous release for variadic binary column buffers but a similar failure had been missed for variadic text columns.

## 1.0.1

* Fix: `Cursor::fetch_with_truncation_check` did erroneously report truncation errors for `NULL` fields, if the query also triggered any ODBC diagnostic. This remained undedected for a while, because in every situation that there is a truncation, there must also be a diagnostic according to the ODBC standard. However the reverse is not true. In this situation a `NULL` had been erronously treated the same as a `NO_TOTAL` indicator and a false positive has been reported to a user claiming that there is a truncation, there in fact is none.
Expand Down
2 changes: 1 addition & 1 deletion odbc-api/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "odbc-api"
version = "1.0.1"
version = "1.0.2"
authors = ["Markus Klein"]
edition = "2021"
license = "MIT"
Expand Down
5 changes: 1 addition & 4 deletions odbc-api/src/buffers/text_column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,7 @@ where
.iter()
.copied()
.take(num_rows)
.any(|indicator| match Indicator::from_isize(indicator) {
Indicator::Null | Indicator::NoTotal => true,
Indicator::Length(length_in_bytes) => max_bin_length < length_in_bytes,
})
.any(|indicator| Indicator::from_isize(indicator).is_truncated(max_bin_length))
}
}

Expand Down
14 changes: 10 additions & 4 deletions odbc-api/src/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,11 @@ impl<'s> CursorRow<'s> {
///
/// `true` indicates that the value has not been `NULL` and the value has been placed in `buf`.
/// `false` indicates that the value is `NULL`. The buffer is cleared in that case.
pub fn get_wide_text(&mut self, col_or_param_num: u16, buf: &mut Vec<u16>) -> Result<bool, Error> {
pub fn get_wide_text(
&mut self,
col_or_param_num: u16,
buf: &mut Vec<u16>,
) -> Result<bool, Error> {
self.get_variadic::<WideText>(col_or_param_num, buf)
}

Expand Down Expand Up @@ -198,8 +202,7 @@ impl<'s> CursorRow<'s> {
let old_len = buf.len();
// Use an exponential strategy for increasing buffer size.
buf.resize(old_len * 2, K::ZERO);
let buf_extend =
&mut buf[(old_len - K::TERMINATING_ZEROES)..];
let buf_extend = &mut buf[(old_len - K::TERMINATING_ZEROES)..];
target = VarCell::<&mut [K::Element], K>::from_buffer(
buf_extend,
Indicator::NoTotal,
Expand All @@ -224,7 +227,10 @@ impl<'s> CursorRow<'s> {
let old_len = buf.len();
buf.resize(old_len + still_missing, K::ZERO);
let buf_extend = &mut buf[(old_len - K::TERMINATING_ZEROES)..];
target = VarCell::<&mut [K::Element], K>::from_buffer(buf_extend, Indicator::NoTotal);
target = VarCell::<&mut [K::Element], K>::from_buffer(
buf_extend,
Indicator::NoTotal,
);
}
}
// Fetch binary data into buffer.
Expand Down
133 changes: 68 additions & 65 deletions odbc-api/src/narrow.rs
Original file line number Diff line number Diff line change
@@ -1,65 +1,68 @@
use crate::{IntoParameter, parameter::{VarCharSlice, VarCharBox}};

/// Newtype wrapper intend to be used around `String`s or `str` slices to bind them always as narrow
/// text independent of wether the `narrow` feature is set or not.
pub struct Narrow<T>(pub T);

impl<'a> IntoParameter for Narrow<&'a str> {
type Parameter = VarCharSlice<'a>;

fn into_parameter(self) -> Self::Parameter {
VarCharSlice::new(self.0.as_bytes())
}
}

impl<'a> IntoParameter for Narrow<Option<&'a str>> {
type Parameter = VarCharSlice<'a>;

fn into_parameter(self) -> Self::Parameter {
match self.0 {
Some(str) => Narrow(str).into_parameter(),
None => VarCharSlice::NULL,
}
}
}

impl<'a> IntoParameter for Option<Narrow<&'a str>> {
type Parameter = VarCharSlice<'a>;

fn into_parameter(self) -> Self::Parameter {
match self {
Some(str) => Narrow(str.0).into_parameter(),
None => VarCharSlice::NULL,
}
}
}

impl IntoParameter for Narrow<String> {
type Parameter = VarCharBox;

fn into_parameter(self) -> Self::Parameter {
VarCharBox::from_string(self.0)
}
}

impl IntoParameter for Narrow<Option<String>> {
type Parameter = VarCharBox;

fn into_parameter(self) -> Self::Parameter {
match self.0 {
Some(str) => Narrow(str).into_parameter(),
None => VarCharBox::null(),
}
}
}

impl IntoParameter for Option<Narrow<String>> {
type Parameter = VarCharBox;

fn into_parameter(self) -> Self::Parameter {
match self {
Some(str) => Narrow(str.0).into_parameter(),
None => VarCharBox::null(),
}
}
}
use crate::{
parameter::{VarCharBox, VarCharSlice},
IntoParameter,
};

/// Newtype wrapper intend to be used around `String`s or `str` slices to bind them always as narrow
/// text independent of wether the `narrow` feature is set or not.
pub struct Narrow<T>(pub T);

impl<'a> IntoParameter for Narrow<&'a str> {
type Parameter = VarCharSlice<'a>;

fn into_parameter(self) -> Self::Parameter {
VarCharSlice::new(self.0.as_bytes())
}
}

impl<'a> IntoParameter for Narrow<Option<&'a str>> {
type Parameter = VarCharSlice<'a>;

fn into_parameter(self) -> Self::Parameter {
match self.0 {
Some(str) => Narrow(str).into_parameter(),
None => VarCharSlice::NULL,
}
}
}

impl<'a> IntoParameter for Option<Narrow<&'a str>> {
type Parameter = VarCharSlice<'a>;

fn into_parameter(self) -> Self::Parameter {
match self {
Some(str) => Narrow(str.0).into_parameter(),
None => VarCharSlice::NULL,
}
}
}

impl IntoParameter for Narrow<String> {
type Parameter = VarCharBox;

fn into_parameter(self) -> Self::Parameter {
VarCharBox::from_string(self.0)
}
}

impl IntoParameter for Narrow<Option<String>> {
type Parameter = VarCharBox;

fn into_parameter(self) -> Self::Parameter {
match self.0 {
Some(str) => Narrow(str).into_parameter(),
None => VarCharBox::null(),
}
}
}

impl IntoParameter for Option<Narrow<String>> {
type Parameter = VarCharBox;

fn into_parameter(self) -> Self::Parameter {
match self {
Some(str) => Narrow(str.0).into_parameter(),
None => VarCharBox::null(),
}
}
}
35 changes: 25 additions & 10 deletions odbc-api/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use odbc_api::{
Blob, BlobRead, BlobSlice, VarBinaryArray, VarCharArray, VarCharSlice, WithDataType,
},
sys, Bit, ColumnDescription, Connection, ConnectionOptions, Cursor, DataType, Error, InOut,
IntoParameter, Nullability, Nullable, Out, ResultSetMetadata, U16Str, U16String, Narrow,
IntoParameter, Narrow, Nullability, Nullable, Out, ResultSetMetadata, U16Str, U16String,
};
use std::{
ffi::CString,
Expand Down Expand Up @@ -1462,23 +1462,32 @@ fn bind_narrow_parameter_to_varchar(profile: &Profile) {
.unwrap();
conn.execute(&insert_sql, &Some(Narrow("Hello")).into_parameter())
.unwrap();
conn.execute(&insert_sql, &None::<Narrow::<&str>>.into_parameter())
conn.execute(&insert_sql, &None::<Narrow<&str>>.into_parameter())
.unwrap();
// String
conn.execute(&insert_sql, &Narrow("Hello".to_string()).into_parameter())
.unwrap();
// Option String
conn.execute(&insert_sql, &Narrow(Some("Hello".to_string())).into_parameter())
.unwrap();
conn.execute(
&insert_sql,
&Narrow(Some("Hello".to_string())).into_parameter(),
)
.unwrap();
conn.execute(&insert_sql, &Narrow(None::<String>).into_parameter())
.unwrap();
conn.execute(&insert_sql, &Some(Narrow("Hello".to_string())).into_parameter())
.unwrap();
conn.execute(&insert_sql, &None::<Narrow::<String>>.into_parameter())
conn.execute(
&insert_sql,
&Some(Narrow("Hello".to_string())).into_parameter(),
)
.unwrap();
conn.execute(&insert_sql, &None::<Narrow<String>>.into_parameter())
.unwrap();

let actual = table.content_as_string(&conn);
assert_eq!("Hello\nHello\nNULL\nHello\nNULL\nHello\nHello\nNULL\nHello\nNULL", actual);
assert_eq!(
"Hello\nHello\nNULL\nHello\nNULL\nHello\nHello\nNULL\nHello\nNULL",
actual
);
}

#[test_case(MSSQL; "Microsoft SQL Server")]
Expand Down Expand Up @@ -3483,7 +3492,10 @@ fn detect_truncated_output_in_bulk_fetch(profile: &Profile) {
let query = format!("SELECT a FROM {table_name}");
let cursor = conn.execute(&query, ()).unwrap().unwrap();
let mut cursor = cursor.bind_buffer(buffer).unwrap();
assert!(matches!(cursor.fetch_with_truncation_check(true), Err(Error::TooLargeValueForBuffer)))
assert!(matches!(
cursor.fetch_with_truncation_check(true),
Err(Error::TooLargeValueForBuffer)
))
}

#[test_case(MSSQL; "Microsoft SQL Server")]
Expand Down Expand Up @@ -3855,7 +3867,10 @@ fn cursor_get_text_from_text(profile: &Profile) {
conn.execute(&insert_sql, &text.into_parameter()).unwrap();

// When
let mut cursor = conn.execute(&table.sql_all_ordered_by_id(), ()).unwrap().unwrap();
let mut cursor = conn
.execute(&table.sql_all_ordered_by_id(), ())
.unwrap()
.unwrap();
let mut row = cursor.next_row().unwrap().unwrap();
let mut buffer = Vec::new();
row.get_text(1, &mut buffer).unwrap();
Expand Down
4 changes: 2 additions & 2 deletions odbcsv/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "odbcsv"
version = "1.0.0"
version = "1.0.1"
authors = ["Markus Klein"]
edition = "2021"
license = "MIT"
Expand Down Expand Up @@ -29,7 +29,7 @@ readme = "Readme.md"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
odbc-api = { version = "1.0.1", path = "../odbc-api" }
odbc-api = { version = "1.0.2", path = "../odbc-api" }
csv = "1.2.2"
anyhow = "1.0.75"
stderrlog = "0.5.4"
Expand Down
4 changes: 4 additions & 0 deletions odbcsv/Changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## 1.0.1

* Fixed an issue which could cause errouneous report of truncation in presence of other warnings. This has been fixed in the previous release for variadic binary column buffers but a similar failure had been missed for variadic text columns.

## 1.0.0

* Updated dependencies
Expand Down

0 comments on commit 97ec441

Please sign in to comment.