Include the name of the column in InvalidColumnType errors #541
Conversation
| @@ -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()), | |||
There was a problem hiding this comment.
It's dodgy to do idx.to_string() here for the name, but I'm not sure it particualrly matters. I almost made the name an Option on the error type, but decided against since it should always be present outside of this case, which should be rare. I'd be willing to change it to do that again if it would be preferred though.
| 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"); |
There was a problem hiding this comment.
The expect is here instead of a panic inside the inner stmt's column_name mostly since I debated exposing this publicly, which would have had column_name return a Option<&str>. I can change that if you want.
thomcc
force-pushed
the
include-column-name-in-type-error
branch
from
8ca8ac8
to
4356f5a
Compare
|
I will try to do a release tomorrow. |
Ah sure, I will. I meant to the last time you suggested but forgot. |
|
Thanks a ton for cutting the release @gwenn! |
We're hitting this in the wild on firefox-ios and knowing that some query has a Null for column 3 is... not particularly actionable. The column name would at least give us a starting place.
I've also made it perform bounds checking on the index in column_name, even though it just panics on out of bounds for now, and refactored that code a little.
It would be nice to get this in a release after it lands, but if that ends up not being possible timing-wise I can just point at my fork.