Merge pull request #541 from thomcc/include-column-name-in-type-error

Include the name of the column in InvalidColumnType errors

src/column.rs

@@ -27,8 +27,7 @@ impl Statement<'_> {
         let n = self.column_count();
         let mut cols = Vec::with_capacity(n as usize);
         for i in 0..n {
-            let slice = self.stmt.column_name(i);
+            let s = self.column_name(i);
-            let s = str::from_utf8(slice.to_bytes()).unwrap();
             cols.push(s);
         }
         cols
@@ -40,6 +39,13 @@ impl Statement<'_> {
         self.stmt.column_count()
     }
 
+    pub(crate) fn column_name(&self, col: usize) -> &str {
+        // Just panic if the bounds are wrong for now, we never call this
+        // without checking first.
+        let slice = self.stmt.column_name(col).expect("Column out of bounds");
+        str::from_utf8(slice.to_bytes()).unwrap()
+    }
+
     /// Returns the column index in the result set for a given column name.
     ///
     /// If there is no AS clause then the name of the column is unspecified and
@@ -53,7 +59,9 @@ impl Statement<'_> {
         let bytes = name.as_bytes();
         let n = self.column_count();
         for i in 0..n {
-            if bytes.eq_ignore_ascii_case(self.stmt.column_name(i).to_bytes()) {
+            // Note: `column_name` is only fallible if `i` is out of bounds,
+            // which we've already checked.
+            if bytes.eq_ignore_ascii_case(self.stmt.column_name(i).unwrap().to_bytes()) {
                 return Ok(i);
             }
         }
@@ -65,8 +73,7 @@ impl Statement<'_> {
         let n = self.column_count();
         let mut cols = Vec::with_capacity(n as usize);
         for i in 0..n {
-            let slice = self.stmt.column_name(i);
+            let name = self.column_name(i);
-            let name = str::from_utf8(slice.to_bytes()).unwrap();
             let slice = self.stmt.column_decltype(i);
             let decl_type = slice.map(|s| str::from_utf8(s.to_bytes()).unwrap());
             cols.push(Column { name, decl_type });
@@ -125,4 +132,38 @@ mod test {
             &[Some("text"), Some("text"), Some("text"),]
         );
     }
+
+    #[test]
+    fn test_column_name_in_error() {
+        use crate::{types::Type, Error};
+        let db = Connection::open_in_memory().unwrap();
+        db.execute_batch(
+            "BEGIN;
+             CREATE TABLE foo(x INTEGER, y TEXT);
+             INSERT INTO foo VALUES(4, NULL);
+             END;").unwrap();
+        let mut stmt = db.prepare("SELECT x as renamed, y FROM foo").unwrap();
+        let mut rows = stmt.query(crate::NO_PARAMS).unwrap();
+        let row = rows.next().unwrap().unwrap();
+        match row.get::<_, String>(0).unwrap_err() {
+            Error::InvalidColumnType(idx, name, ty) => {
+                assert_eq!(idx, 0);
+                assert_eq!(name, "renamed");
+                assert_eq!(ty, Type::Integer);
+            }
+            e => {
+                panic!("Unexpected error type: {:?}", e);
+            }
+        }
+        match row.get::<_, String>("y").unwrap_err() {
+            Error::InvalidColumnType(idx, name, ty) => {
+                assert_eq!(idx, 1);
+                assert_eq!(name, "y");
+                assert_eq!(ty, Type::Null);
+            }
+            e => {
+                panic!("Unexpected error type: {:?}", e);
+            }
+        }
+    }
 }

src/error.rs

@@ -59,7 +59,7 @@ pub enum Error {
     /// Error when the value of a particular column is requested, but the type
     /// of the result in that column cannot be converted to the requested
     /// Rust type.
-    InvalidColumnType(usize, Type),
+    InvalidColumnType(usize, String, Type),
 
     /// Error when a query that was expected to insert one row did not insert
     /// any or insert many.
@@ -117,8 +117,8 @@ impl PartialEq for Error {
             (Error::QueryReturnedNoRows, Error::QueryReturnedNoRows) => true,
             (Error::InvalidColumnIndex(i1), Error::InvalidColumnIndex(i2)) => i1 == i2,
             (Error::InvalidColumnName(n1), Error::InvalidColumnName(n2)) => n1 == n2,
-            (Error::InvalidColumnType(i1, t1), Error::InvalidColumnType(i2, t2)) => {
+            (Error::InvalidColumnType(i1, n1, t1), Error::InvalidColumnType(i2, n2, t2)) => {
-                i1 == i2 && t1 == t2
+                i1 == i2 && t1 == t2 && n1 == n2
             }
             (Error::StatementChangedRows(n1), Error::StatementChangedRows(n2)) => n1 == n2,
             #[cfg(feature = "functions")]
@@ -182,8 +182,8 @@ impl fmt::Display for Error {
             Error::QueryReturnedNoRows => write!(f, "Query returned no rows"),
             Error::InvalidColumnIndex(i) => write!(f, "Invalid column index: {}", i),
             Error::InvalidColumnName(ref name) => write!(f, "Invalid column name: {}", name),
-            Error::InvalidColumnType(i, ref t) => {
+            Error::InvalidColumnType(i, ref name, ref t) => {
-                write!(f, "Invalid column type {} at index: {}", t, i)
+                write!(f, "Invalid column type {} at index: {}, name: {}", t, i, name)
             }
             Error::StatementChangedRows(i) => write!(f, "Query changed {} rows", i),
 
@@ -229,7 +229,7 @@ impl error::Error for Error {
             Error::QueryReturnedNoRows => "query returned no rows",
             Error::InvalidColumnIndex(_) => "invalid column index",
             Error::InvalidColumnName(_) => "invalid column name",
-            Error::InvalidColumnType(_, _) => "invalid column type",
+            Error::InvalidColumnType(_, _, _) => "invalid column type",
             Error::StatementChangedRows(_) => "query inserted zero or more than one row",
 
             #[cfg(feature = "functions")]
@@ -262,7 +262,7 @@ impl error::Error for Error {
             | Error::QueryReturnedNoRows
             | Error::InvalidColumnIndex(_)
             | Error::InvalidColumnName(_)
-            | Error::InvalidColumnType(_, _)
+            | Error::InvalidColumnType(_, _, _)
             | Error::InvalidPath(_)
             | Error::StatementChangedRows(_)
             | Error::InvalidQuery => None,

src/lib.rs

@@ -1476,7 +1476,7 @@ mod test {
                 .collect();
 
             match bad_type.unwrap_err() {
-                Error::InvalidColumnType(_, _) => (),
+                Error::InvalidColumnType(_, _, _) => (),
                 err => panic!("Unexpected error {}", err),
             }
 
@@ -1531,7 +1531,7 @@ mod test {
                 .collect();
 
             match bad_type.unwrap_err() {
-                CustomError::Sqlite(Error::InvalidColumnType(_, _)) => (),
+                CustomError::Sqlite(Error::InvalidColumnType(_, _, _)) => (),
                 err => panic!("Unexpected error {}", err),
             }
 
@@ -1588,7 +1588,7 @@ mod test {
             });
 
             match bad_type.unwrap_err() {
-                CustomError::Sqlite(Error::InvalidColumnType(_, _)) => (),
+                CustomError::Sqlite(Error::InvalidColumnType(_, _, _)) => (),
                 err => panic!("Unexpected error {}", err),
             }
 

src/raw_statement.rs

@@ -37,8 +37,18 @@ impl RawStatement {
         }
     }
 
-    pub fn column_name(&self, idx: usize) -> &CStr {
+    pub fn column_name(&self, idx: usize) -> Option<&CStr> {
-        unsafe { CStr::from_ptr(ffi::sqlite3_column_name(self.0, idx as c_int)) }
+        let idx = idx as c_int;
+        if idx < 0 || idx >= self.column_count() as c_int {
+            return None;
+        }
+        unsafe {
+            let ptr = ffi::sqlite3_column_name(self.0, idx);
+            // If ptr is null here, it's an OOM, so there's probably nothing
+            // meaningful we can do. Just assert instead of returning None.
+            assert!(!ptr.is_null(), "Null pointer from sqlite3_column_name: Out of memory?");
+            Some(CStr::from_ptr(ptr))
+        }
     }
 
     pub fn step(&self) -> c_int {

src/row.rs

@@ -223,15 +223,15 @@ impl<'stmt> Row<'stmt> {
         let idx = idx.idx(self.stmt)?;
         let value = self.stmt.value_ref(idx);
         FromSql::column_result(value).map_err(|err| match err {
-            FromSqlError::InvalidType => Error::InvalidColumnType(idx, value.data_type()),
+            FromSqlError::InvalidType => Error::InvalidColumnType(idx, self.stmt.column_name(idx).into(), value.data_type()),
             FromSqlError::OutOfRange(i) => Error::IntegralValueOutOfRange(idx, i),
             FromSqlError::Other(err) => {
                 Error::FromSqlConversionFailure(idx as usize, value.data_type(), err)
             }
             #[cfg(feature = "i128_blob")]
-            FromSqlError::InvalidI128Size(_) => Error::InvalidColumnType(idx, value.data_type()),
+            FromSqlError::InvalidI128Size(_) => Error::InvalidColumnType(idx, self.stmt.column_name(idx).into(), value.data_type()),
             #[cfg(feature = "uuid")]
-            FromSqlError::InvalidUuidSize(_) => Error::InvalidColumnType(idx, value.data_type()),
+            FromSqlError::InvalidUuidSize(_) => Error::InvalidColumnType(idx, self.stmt.column_name(idx).into(), value.data_type()),
         })
     }
 

src/types/mod.rs

@@ -229,7 +229,7 @@ mod test {
     fn test_mismatched_types() {
         fn is_invalid_column_type(err: Error) -> bool {
             match err {
-                Error::InvalidColumnType(_, _) => true,
+                Error::InvalidColumnType(_, _, _) => true,
                 _ => false,
             }
         }

src/vtab/mod.rs

@@ -472,7 +472,7 @@ impl Values<'_> {
             }
             FromSqlError::OutOfRange(i) => Error::IntegralValueOutOfRange(idx, i),
             #[cfg(feature = "i128_blob")]
-            FromSqlError::InvalidI128Size(_) => Error::InvalidColumnType(idx, value.data_type()),
+            FromSqlError::InvalidI128Size(_) => Error::InvalidColumnType(idx, idx.to_string(), value.data_type()),
             #[cfg(feature = "uuid")]
             FromSqlError::InvalidUuidSize(_) => {
                 Error::FromSqlConversionFailure(idx, value.data_type(), Box::new(err))