Skip to content

Commit c56a162

Browse files
committed
Provide FromSql impls for *const str and *const [u8]
There are several impls which could be composed on impls for `&str` and `&[u8]` if they were available. Unfortunately, providing those impls is impossible in Diesel 1.0. We would need a lifetime on `FromSql`, changing the signature to this: ```rust trait FromSql<'a, ST, DB> { fn from_sql(bytes: Option<&'a DB::RawValue>) -> ... } ``` While we can't provide impls for `&str` and `&[u8]`, we can provide impls for `*const` versions of both. The docs about how long the pointer is valid for *do* get rendered, and since dereferencing this pointers requires unsafe code, we can reasonably assume that anyone who uses these impls has looked at the docs for the assumed invaraints. I've updated several of our built-in impls that just do string parsing to use these impls, to demonstrate why they're useful. Keep in mind that outside of Diesel it is impossible to reference `SqliteValue`. While the impls I've updated are backend specific, it is also possible to use these impls to write backend agnostic implementations. Fixes diesel-rs#1365.
1 parent dc73c4f commit c56a162

7 files changed

Lines changed: 97 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
5050
* `helper_types` now contains a type for every method defined in
5151
`expression_methods`, and every function in `dsl`.
5252

53+
* Added `FromSql` impls for `*const str` and `*const [u8]` everywhere that
54+
`String` and `Vec` are supported. These impls do not allocate, and are
55+
intended for use by other impls which need to parse a string or bytes, and
56+
don't want to allocate. These impls should never be used outside of another
57+
`FromSql` impl.
58+
5359
### Deprecated
5460

5561
* *IMPORTANT NOTE* Do to [several][rust-deprecation-bug-1]

diesel/src/mysql/types/numeric.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@ pub mod bigdecimal {
55
use self::bigdecimal::BigDecimal;
66
use std::io::prelude::*;
77

8+
use backend::Backend;
89
use deserialize::{self, FromSql};
910
use mysql::Mysql;
1011
use serialize::{self, IsNull, Output, ToSql};
11-
use sql_types::Numeric;
12+
use sql_types::{Binary, Numeric};
1213

1314
impl ToSql<Numeric, Mysql> for BigDecimal {
1415
fn to_sql<W: Write>(&self, out: &mut Output<W, Mysql>) -> serialize::Result {
@@ -19,8 +20,9 @@ pub mod bigdecimal {
1920
}
2021

2122
impl FromSql<Numeric, Mysql> for BigDecimal {
22-
fn from_sql(bytes: Option<&[u8]>) -> deserialize::Result<Self> {
23-
let bytes = not_none!(bytes);
23+
fn from_sql(bytes: Option<&<Mysql as Backend>::RawValue>) -> deserialize::Result<Self> {
24+
let bytes_ptr = <*const [u8] as FromSql<Binary, Mysql>>::from_sql(bytes)?;
25+
let bytes = unsafe { &*bytes_ptr };
2426
BigDecimal::parse_bytes(bytes, 10)
2527
.ok_or_else(|| Box::from(format!("{:?} is not valid decimal number ", bytes)))
2628
}

diesel/src/sqlite/types/date_and_time/chrono.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,18 @@ extern crate chrono;
33
use std::io::Write;
44
use self::chrono::{NaiveDate, NaiveDateTime, NaiveTime};
55

6+
use backend::Backend;
67
use deserialize::{self, FromSql};
78
use serialize::{self, Output, ToSql};
89
use sqlite::Sqlite;
9-
use sqlite::connection::SqliteValue;
1010
use sql_types::{Date, Text, Time, Timestamp};
1111

1212
const SQLITE_DATE_FORMAT: &str = "%F";
1313

1414
impl FromSql<Date, Sqlite> for NaiveDate {
15-
fn from_sql(value: Option<&SqliteValue>) -> deserialize::Result<Self> {
16-
let text = not_none!(value).read_text();
15+
fn from_sql(value: Option<&<Sqlite as Backend>::RawValue>) -> deserialize::Result<Self> {
16+
let text_ptr = <*const str as FromSql<Date, Sqlite>>::from_sql(value)?;
17+
let text = unsafe { &*text_ptr };
1718
Self::parse_from_str(text, SQLITE_DATE_FORMAT).map_err(Into::into)
1819
}
1920
}
@@ -26,8 +27,9 @@ impl ToSql<Date, Sqlite> for NaiveDate {
2627
}
2728

2829
impl FromSql<Time, Sqlite> for NaiveTime {
29-
fn from_sql(value: Option<&SqliteValue>) -> deserialize::Result<Self> {
30-
let text = not_none!(value).read_text();
30+
fn from_sql(value: Option<&<Sqlite as Backend>::RawValue>) -> deserialize::Result<Self> {
31+
let text_ptr = <*const str as FromSql<Date, Sqlite>>::from_sql(value)?;
32+
let text = unsafe { &*text_ptr };
3133
let valid_time_formats = &[
3234
// Most likely
3335
"%T%.f",
@@ -57,8 +59,9 @@ impl ToSql<Time, Sqlite> for NaiveTime {
5759
}
5860

5961
impl FromSql<Timestamp, Sqlite> for NaiveDateTime {
60-
fn from_sql(value: Option<&SqliteValue>) -> deserialize::Result<Self> {
61-
let text = not_none!(value).read_text();
62+
fn from_sql(value: Option<&<Sqlite as Backend>::RawValue>) -> deserialize::Result<Self> {
63+
let text_ptr = <*const str as FromSql<Date, Sqlite>>::from_sql(value)?;
64+
let text = unsafe { &*text_ptr };
6265

6366
let sqlite_datetime_formats = &[
6467
// Most likely format

diesel/src/sqlite/types/date_and_time/mod.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,12 @@ use sql_types;
99
#[cfg(feature = "chrono")]
1010
mod chrono;
1111

12-
impl FromSql<sql_types::Date, Sqlite> for String {
12+
/// The returned pointer is *only* valid for the lifetime to the argument of
13+
/// `from_sql`. This impl is intended for uses where you want to write a new
14+
/// impl in terms of `String`, but don't want to allocate. We have to return a
15+
/// raw pointer instead of a reference with a lifetime due to the structure of
16+
/// `FromSql`
17+
impl FromSql<sql_types::Date, Sqlite> for *const str {
1318
fn from_sql(value: Option<&SqliteValue>) -> deserialize::Result<Self> {
1419
FromSql::<sql_types::Text, Sqlite>::from_sql(value)
1520
}
@@ -27,7 +32,12 @@ impl ToSql<sql_types::Date, Sqlite> for String {
2732
}
2833
}
2934

30-
impl FromSql<sql_types::Time, Sqlite> for String {
35+
/// The returned pointer is *only* valid for the lifetime to the argument of
36+
/// `from_sql`. This impl is intended for uses where you want to write a new
37+
/// impl in terms of `String`, but don't want to allocate. We have to return a
38+
/// raw pointer instead of a reference with a lifetime due to the structure of
39+
/// `FromSql`
40+
impl FromSql<sql_types::Time, Sqlite> for *const str {
3141
fn from_sql(value: Option<&SqliteValue>) -> deserialize::Result<Self> {
3242
FromSql::<sql_types::Text, Sqlite>::from_sql(value)
3343
}
@@ -45,7 +55,12 @@ impl ToSql<sql_types::Time, Sqlite> for String {
4555
}
4656
}
4757

48-
impl FromSql<sql_types::Timestamp, Sqlite> for String {
58+
/// The returned pointer is *only* valid for the lifetime to the argument of
59+
/// `from_sql`. This impl is intended for uses where you want to write a new
60+
/// impl in terms of `String`, but don't want to allocate. We have to return a
61+
/// raw pointer instead of a reference with a lifetime due to the structure of
62+
/// `FromSql`
63+
impl FromSql<sql_types::Timestamp, Sqlite> for *const str {
4964
fn from_sql(value: Option<&SqliteValue>) -> deserialize::Result<Self> {
5065
FromSql::<sql_types::Text, Sqlite>::from_sql(value)
5166
}

diesel/src/sqlite/types/mod.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,27 @@ use super::Sqlite;
88
use super::connection::SqliteValue;
99
use sql_types;
1010

11-
impl FromSql<sql_types::VarChar, Sqlite> for String {
11+
/// The returned pointer is *only* valid for the lifetime to the argument of
12+
/// `from_sql`. This impl is intended for uses where you want to write a new
13+
/// impl in terms of `String`, but don't want to allocate. We have to return a
14+
/// raw pointer instead of a reference with a lifetime due to the structure of
15+
/// `FromSql`
16+
impl FromSql<sql_types::VarChar, Sqlite> for *const str {
1217
fn from_sql(value: Option<&SqliteValue>) -> deserialize::Result<Self> {
1318
let text = not_none!(value).read_text();
14-
Ok(text.into())
19+
Ok(text as *const _)
1520
}
1621
}
1722

18-
impl FromSql<sql_types::Binary, Sqlite> for Vec<u8> {
23+
/// The returned pointer is *only* valid for the lifetime to the argument of
24+
/// `from_sql`. This impl is intended for uses where you want to write a new
25+
/// impl in terms of `Vec<u8>`, but don't want to allocate. We have to return a
26+
/// raw pointer instead of a reference with a lifetime due to the structure of
27+
/// `FromSql`
28+
impl FromSql<sql_types::Binary, Sqlite> for *const [u8] {
1929
fn from_sql(bytes: Option<&SqliteValue>) -> deserialize::Result<Self> {
2030
let bytes = not_none!(bytes).read_blob();
21-
Ok(bytes.into())
31+
Ok(bytes as *const _)
2232
}
2333
}
2434

diesel/src/type_impls/primitives.rs

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,29 @@ mod foreign_impls {
8383

8484
impl NotNull for () {}
8585

86-
impl<DB: Backend<RawValue = [u8]>> FromSql<sql_types::Text, DB> for String {
87-
fn from_sql(bytes: Option<&[u8]>) -> deserialize::Result<Self> {
88-
let bytes = not_none!(bytes);
89-
String::from_utf8(bytes.into()).map_err(|e| Box::new(e) as Box<Error + Send + Sync>)
86+
impl<ST, DB> FromSql<ST, DB> for String
87+
where
88+
DB: Backend,
89+
*const str: FromSql<ST, DB>,
90+
{
91+
fn from_sql(bytes: Option<&DB::RawValue>) -> deserialize::Result<Self> {
92+
let str_ptr = <*const str as FromSql<ST, DB>>::from_sql(bytes)?;
93+
// We know that the pointer impl will never return null
94+
let string = unsafe { &*str_ptr };
95+
Ok(string.to_owned())
96+
}
97+
}
98+
99+
/// The returned pointer is *only* valid for the lifetime to the argument of
100+
/// `from_sql`. This impl is intended for uses where you want to write a new
101+
/// impl in terms of `String`, but don't want to allocate. We have to return a
102+
/// raw pointer instead of a reference with a lifetime due to the structure of
103+
/// `FromSql`
104+
impl<DB: Backend<RawValue = [u8]>> FromSql<sql_types::Text, DB> for *const str {
105+
fn from_sql(bytes: Option<&DB::RawValue>) -> deserialize::Result<Self> {
106+
use std::str;
107+
let string = str::from_utf8(not_none!(bytes))?;
108+
Ok(string as *const _)
90109
}
91110
}
92111

@@ -108,9 +127,27 @@ where
108127
}
109128
}
110129

111-
impl<DB: Backend<RawValue = [u8]>> FromSql<sql_types::Binary, DB> for Vec<u8> {
130+
impl<ST, DB> FromSql<ST, DB> for Vec<u8>
131+
where
132+
DB: Backend,
133+
*const [u8]: FromSql<ST, DB>,
134+
{
135+
fn from_sql(bytes: Option<&DB::RawValue>) -> deserialize::Result<Self> {
136+
let slice_ptr = <*const [u8] as FromSql<ST, DB>>::from_sql(bytes)?;
137+
// We know that the pointer impl will never return null
138+
let bytes = unsafe { &*slice_ptr };
139+
Ok(bytes.to_owned())
140+
}
141+
}
142+
143+
/// The returned pointer is *only* valid for the lifetime to the argument of
144+
/// `from_sql`. This impl is intended for uses where you want to write a new
145+
/// impl in terms of `Vec<u8>`, but don't want to allocate. We have to return a
146+
/// raw pointer instead of a reference with a lifetime due to the structure of
147+
/// `FromSql`
148+
impl<DB: Backend<RawValue = [u8]>> FromSql<sql_types::Binary, DB> for *const [u8] {
112149
fn from_sql(bytes: Option<&DB::RawValue>) -> deserialize::Result<Self> {
113-
Ok(not_none!(bytes).into())
150+
Ok(not_none!(bytes) as *const _)
114151
}
115152
}
116153

diesel_compile_tests/tests/compile-fail/select_carries_correct_result_type_info.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,5 @@ fn main() {
2020
let ids = select_name.load::<i32>(&connection);
2121
//~^ ERROR the trait bound `i32: diesel::deserialize::FromSql<diesel::sql_types::Text, _>` is not satisfied
2222
let names = select_id.load::<String>(&connection);
23-
//~^ ERROR the trait bound `std::string::String: diesel::deserialize::FromSql<diesel::sql_types::Integer, _>` is not satisfied
23+
//~^ ERROR the trait bound `*const str: diesel::deserialize::FromSql<diesel::sql_types::Integer, _>` is not satisfied
2424
}

0 commit comments

Comments
 (0)