Skip to content

Commit f4511c6

Browse files
committed
Allow inserting Option<T> into columns where the type is not nullable
This is important to allow structs with types like `Option<i32>` for `id` to be used with `Insertable`, since the column has a default. Ideally we would only allow this to work with columns that have a default value. I may make that change in future, but at the moment we do not have a way of knowing which columns have a default. I am OK with the additional incorrect queries that this allows, as they were already possible. There's no reason that `struct NewUser { name: Option<String>, hair_color: Option<String>, }` shouldn't work if `struct NewUser { hair_color: Option<String> }` works, as they both generate incorrect queries (name is not null and has no default in this example). I've documented the change as only affecting columns with a default, as we reserve the right to disallow using `Option<T>` with columns that have no default value in the future. Ideally before 1.0 I'd like to replace this with a solution that doesn't leak the `IntoNullable::SqlType` constraint in the public API. Fixes diesel-rs#554.
1 parent 8451b93 commit f4511c6

6 files changed

Lines changed: 44 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
1717

1818
* The `persistable` module has been renamed to `insertable`.
1919

20+
### Fixed
21+
22+
* `#[derive(Insertable)]` allows fields of type `Option<T>` to be used with
23+
columns that are not null if they have a default value.
24+
2025
## [0.9.1] - 2016-12-09
2126

2227
### Fixed

diesel/src/expression/helper_types.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,7 @@ pub type NotBetween<Lhs, Rhs> = super::predicates::NotBetween<Lhs,
4040
pub use super::predicates::{IsNull, IsNotNull, Asc, Desc};
4141
#[doc(inline)]
4242
pub use super::array_comparison::EqAny;
43+
44+
#[doc(hidden)]
45+
pub type AsNullableExpr<Item, TargetExpr> = AsExprOf<Item,
46+
<SqlTypeOf<TargetExpr> as types::IntoNullable>::Nullable>;

diesel/src/insertable.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use expression::Expression;
55
use result::QueryResult;
66
use query_builder::{QueryBuilder, BuildQueryResult};
77
use query_source::{Table, Column};
8+
use types::IntoNullable;
89

910
/// Represents that a structure can be used to to insert a new row into the
1011
/// database. This is automatically implemented for `&[T]` and `&Vec<T>` for
@@ -32,7 +33,8 @@ pub trait InsertValues<DB: Backend> {
3233
#[derive(Debug, Copy, Clone)]
3334
pub enum ColumnInsertValue<Col, Expr> where
3435
Col: Column,
35-
Expr: Expression<SqlType=Col::SqlType>,
36+
Col::SqlType: IntoNullable,
37+
Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>,
3638
{
3739
Expression(Col, Expr),
3840
Default(Col),

diesel/src/macros/insertable.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ macro_rules! impl_Insertable {
163163
($(
164164
$crate::insertable::ColumnInsertValue<
165165
$table_name::$column_name,
166-
$crate::expression::helper_types::AsExpr<
166+
$crate::expression::helper_types::AsNullableExpr<
167167
&'insert $field_ty,
168168
$table_name::$column_name,
169169
>,
@@ -173,7 +173,7 @@ macro_rules! impl_Insertable {
173173
type Values = ($(
174174
$crate::insertable::ColumnInsertValue<
175175
$table_name::$column_name,
176-
$crate::expression::helper_types::AsExpr<
176+
$crate::expression::helper_types::AsNullableExpr<
177177
&'insert $field_ty,
178178
$table_name::$column_name,
179179
>,
@@ -184,6 +184,7 @@ macro_rules! impl_Insertable {
184184
fn values(self) -> Self::Values {
185185
use $crate::expression::{AsExpression, Expression};
186186
use $crate::insertable::ColumnInsertValue;
187+
use $crate::types::IntoNullable;
187188
let $self_to_columns = *self;
188189
($(
189190
Insertable_column_expr!($table_name::$column_name, $column_name, $field_kind)
@@ -222,7 +223,7 @@ macro_rules! Insertable_column_expr {
222223
($column:path, $field_access:expr, regular) => {
223224
ColumnInsertValue::Expression(
224225
$column,
225-
AsExpression::<<$column as Expression>::SqlType>
226+
AsExpression::<<<$column as Expression>::SqlType as IntoNullable>::Nullable>
226227
::as_expression($field_access),
227228
)
228229
};

diesel/src/types/impls/tuples.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ macro_rules! tuple_impls {
111111
DB: Backend + SupportsDefaultKeyword,
112112
Tab: Table,
113113
$($T: Column<Table=Tab>,)+
114-
$($ST: Expression<SqlType=$T::SqlType> + QueryFragment<DB>,)+
114+
$($T::SqlType: IntoNullable,)+
115+
$($ST: Expression<SqlType=<$T::SqlType as IntoNullable>::Nullable> + QueryFragment<DB>,)+
115116
{
116117
fn column_names(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
117118
$(
@@ -158,7 +159,8 @@ macro_rules! tuple_impls {
158159
for ($(ColumnInsertValue<$T, $ST>,)+) where
159160
Tab: Table,
160161
$($T: Column<Table=Tab>,)+
161-
$($ST: Expression<SqlType=$T::SqlType> + QueryFragment<::sqlite::Sqlite>,)+
162+
$($T::SqlType: IntoNullable,)+
163+
$($ST: Expression<SqlType=<$T::SqlType as IntoNullable>::Nullable> + QueryFragment<::sqlite::Sqlite>,)+
162164
{
163165
#[allow(unused_assignments)]
164166
fn column_names(&self, out: &mut ::sqlite::SqliteQueryBuilder) -> BuildQueryResult {

diesel_tests/tests/annotations.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,3 +215,27 @@ fn derive_identifiable_with_composite_pk() {
215215
assert_eq!((&2, &3), foo1.id());
216216
assert_eq!((&6, &7), foo2.id());
217217
}
218+
219+
#[test]
220+
fn derive_insertable_with_option_for_not_null_field_with_default() {
221+
#[derive(Insertable)]
222+
#[table_name="users"]
223+
struct NewUser {
224+
id: Option<i32>,
225+
name: &'static str,
226+
}
227+
228+
let conn = connection();
229+
let data = vec![
230+
NewUser { id: None, name: "Jim" },
231+
NewUser { id: Some(123), name: "Bob" },
232+
];
233+
assert_eq!(Ok(2), insert(&data).into(users::table).execute(&conn));
234+
235+
let users = users::table.load::<User>(&conn).unwrap();
236+
let jim = users.iter().find(|u| u.name == "Jim");
237+
let bob = users.iter().find(|u| u.name == "Bob");
238+
239+
assert!(jim.is_some());
240+
assert_eq!(Some(&User::new(123, "Bob")), bob);
241+
}

0 commit comments

Comments
 (0)