Skip to content

Commit

Permalink
Remove the ability to have Nullable<Nullable<T>> as a type
Browse files Browse the repository at this point in the history
This is part of a greater effort to clean up some of the messier bits of
our type system. Unfortunately, we still can't implement something both
for null and not null in a non-overlapping way most of the time, as we
still will bump into rust-lang/rust#20400.

Otherwise we could write something like:

```rust
impl<T, ST> Expression for Something<T> where
    T: Expression<SqlType=ST>,
    ST: NotNull,
{
}

impl<T, ST> Expression for Something<T> where
    T: Expression<SqlType=Nullable<ST>>,
    ST: NotNull,
{
}
```

While these clearly do not overlap, rust treats them as overlapping.
Unfortunately this is just one more type that we need to implement for
new SQL types added in the future. I can't just make `IntoNullable`
be a requirement of `NativeSqlType`, as it would require specifying the
nullable case when packing it into a trait object.

`NotNull` can just be replaced with an OIBIT in the future.
  • Loading branch information
sgrif committed Jan 18, 2016
1 parent 4120ecf commit 29280bf
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 23 deletions.
4 changes: 2 additions & 2 deletions diesel/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,8 @@ macro_rules! select_column_inner {

impl $crate::expression::SelectableExpression<
$crate::query_source::LeftOuterJoinSource<$child::table, $parent::table>,
$crate::types::Nullable<
<$parent::$column_name as $crate::Expression>::SqlType>,
<<$parent::$column_name as $crate::Expression>::SqlType
as $crate::types::IntoNullable>::Nullable,
> for $parent::$column_name
{
}
Expand Down
9 changes: 5 additions & 4 deletions diesel/src/query_source/joins.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::{QuerySource, Table};
use query_builder::*;
use expression::SelectableExpression;
use types::Nullable;
use types::IntoNullable;

#[derive(Debug, Clone, Copy)]
#[doc(hidden)]
Expand Down Expand Up @@ -80,14 +80,15 @@ impl<Left, Right> QuerySource for LeftOuterJoinSource<Left, Right> where
impl<Left, Right> AsQuery for LeftOuterJoinSource<Left, Right> where
Left: Table + JoinTo<Right>,
Right: Table,
Right::SqlType: IntoNullable,
(Left::AllColumns, Right::AllColumns): SelectableExpression<
LeftOuterJoinSource<Left, Right>,
(Left::SqlType, Nullable<Right::SqlType>),
(Left::SqlType, <Right::SqlType as IntoNullable>::Nullable),
>,
{
type SqlType = (Left::SqlType, Nullable<Right::SqlType>);
type SqlType = (Left::SqlType, <Right::SqlType as IntoNullable>::Nullable);
type Query = SelectStatement<
(Left::SqlType, Nullable<Right::SqlType>),
Self::SqlType,
(Left::AllColumns, Right::AllColumns),
Self,
>;
Expand Down
5 changes: 4 additions & 1 deletion diesel/src/types/impls/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::io::Write;

use query_source::Queryable;
use super::option::UnexpectedNullError;
use types::{NativeSqlType, FromSql, ToSql, Array, IsNull};
use types::{NativeSqlType, FromSql, ToSql, Array, IsNull, NotNull};

impl<T: NativeSqlType> NativeSqlType for Array<T> {
fn oid(&self) -> u32 {
Expand All @@ -22,6 +22,9 @@ impl<T: NativeSqlType> NativeSqlType for Array<T> {
}
}

impl<T: NativeSqlType> NotNull for Array<T> {
}

impl<T, ST> FromSql<Array<ST>> for Vec<T> where
T: FromSql<ST>,
ST: NativeSqlType,
Expand Down
3 changes: 3 additions & 0 deletions diesel/src/types/impls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ macro_rules! primitive_impls {
types::$Source
}
}

impl types::NotNull for types::$Source {
}
)+
queryable_impls!($($Source -> $Target),+,);
expression_impls!($($Source -> $Target),+,);
Expand Down
14 changes: 7 additions & 7 deletions diesel/src/types/impls/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use expression::bound::Bound;
use std::error::Error;
use std::fmt;
use std::io::Write;
use types::{NativeSqlType, FromSql, FromSqlRow, Nullable, ToSql, IsNull};
use types::{NativeSqlType, FromSql, FromSqlRow, Nullable, ToSql, IsNull, NotNull};

impl<T: NativeSqlType> NativeSqlType for Nullable<T> {
impl<T: NativeSqlType + NotNull> NativeSqlType for Nullable<T> {
fn oid(&self) -> u32 {
self.0.oid()
}
Expand All @@ -22,7 +22,7 @@ impl<T: NativeSqlType> NativeSqlType for Nullable<T> {

impl<T, ST> FromSql<Nullable<ST>> for Option<T> where
T: FromSql<ST>,
ST: NativeSqlType,
ST: NativeSqlType + NotNull,
{
fn from_sql(bytes: Option<&[u8]>) -> Result<Self, Box<Error>> {
match bytes {
Expand All @@ -35,7 +35,7 @@ impl<T, ST> FromSql<Nullable<ST>> for Option<T> where
impl<T, ST> Queryable<Nullable<ST>> for Option<T> where
T: Queryable<ST>,
Option<T::Row>: FromSqlRow<Nullable<ST>>,
ST: NativeSqlType,
ST: NativeSqlType + NotNull,
{
type Row = Option<T::Row>;

Expand All @@ -46,7 +46,7 @@ impl<T, ST> Queryable<Nullable<ST>> for Option<T> where

impl<T, ST> ToSql<Nullable<ST>> for Option<T> where
T: ToSql<ST>,
ST: NativeSqlType,
ST: NativeSqlType + NotNull,
{
fn to_sql<W: Write>(&self, out: &mut W) -> Result<IsNull, Box<Error>> {
if let &Some(ref value) = self {
Expand All @@ -59,7 +59,7 @@ impl<T, ST> ToSql<Nullable<ST>> for Option<T> where

impl<T, ST> AsExpression<Nullable<ST>> for Option<T> where
Option<T>: ToSql<Nullable<ST>>,
ST: NativeSqlType,
ST: NativeSqlType + NotNull,
{
type Expression = Bound<Nullable<ST>, Self>;

Expand All @@ -70,7 +70,7 @@ impl<T, ST> AsExpression<Nullable<ST>> for Option<T> where

impl<'a, T, ST> AsExpression<Nullable<ST>> for &'a Option<T> where
Option<T>: ToSql<Nullable<ST>>,
ST: NativeSqlType,
ST: NativeSqlType + NotNull,
{
type Expression = Bound<Nullable<ST>, Self>;

Expand Down
4 changes: 3 additions & 1 deletion diesel/src/types/impls/primitives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use data_types::PgNumeric;
use expression::bound::Bound;
use expression::{Expression, AsExpression};
use super::option::UnexpectedNullError;
use types::{NativeSqlType, FromSql, ToSql, IsNull};
use types::{NativeSqlType, FromSql, ToSql, IsNull, NotNull};
use {Queryable, types};

primitive_impls! {
Expand Down Expand Up @@ -48,6 +48,8 @@ impl NativeSqlType for () {
}
}

impl NotNull for () {}

impl FromSql<types::Bool> for bool {
fn from_sql(bytes: Option<&[u8]>) -> Result<Self, Box<Error>> {
match bytes {
Expand Down
9 changes: 6 additions & 3 deletions diesel/src/types/impls/tuples.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use query_builder::{Changeset, QueryBuilder, BuildQueryResult};
use query_source::{QuerySource, Queryable, Table, Column};
use row::Row;
use std::error::Error;
use types::{NativeSqlType, FromSqlRow, ToSql, Nullable};
use types::{NativeSqlType, FromSqlRow, ToSql, Nullable, IntoNullable, NotNull};

// FIXME(https://github.com/rust-lang/rust/issues/19630) Remove this work-around
macro_rules! e {
Expand Down Expand Up @@ -32,6 +32,9 @@ macro_rules! tuple_impls {
}
}

impl<$($T: NativeSqlType),+> NotNull for ($($T,)+) {
}

impl<$($T),+,$($ST),+> FromSqlRow<($($ST,)+)> for ($($T,)+) where
$($T: FromSqlRow<$ST>),+,
$($ST: NativeSqlType),+
Expand Down Expand Up @@ -115,8 +118,8 @@ macro_rules! tuple_impls {
impl<$($T),+, $($ST),+, QS>
SelectableExpression<QS, Nullable<($($ST,)+)>>
for ($($T,)+) where
$($ST: NativeSqlType),+,
$($T: SelectableExpression<QS, Nullable<$ST>>),+,
$($ST: NativeSqlType + IntoNullable),+,
$($T: SelectableExpression<QS, $ST::Nullable, SqlType=$ST>),+,
($($T,)+): Expression,
{
}
Expand Down
17 changes: 16 additions & 1 deletion diesel/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub type BigSerial = BigInt;
#[derive(Clone, Copy, Default)] pub struct Time;
#[derive(Clone, Copy, Default)] pub struct Timestamp;

#[derive(Clone, Copy, Default)] pub struct Nullable<T: NativeSqlType>(T);
#[derive(Clone, Copy, Default)] pub struct Nullable<T: NativeSqlType + NotNull>(T);
#[derive(Clone, Copy, Default)] pub struct Array<T: NativeSqlType>(T);

pub trait NativeSqlType {
Expand All @@ -63,6 +63,21 @@ pub trait NativeSqlType {
fn new() -> Self where Self: Sized;
}

pub trait NotNull {
}

pub trait IntoNullable {
type Nullable: NativeSqlType;
}

impl<T: NativeSqlType + NotNull> IntoNullable for T {
type Nullable = Nullable<T>;
}

impl<T: NativeSqlType + NotNull> IntoNullable for Nullable<T> {
type Nullable = Nullable<T>;
}

/// How to deserialize a single field of a given type. The input will always be
/// the binary representation, not the text.
pub trait FromSql<A: NativeSqlType>: Sized {
Expand Down
4 changes: 2 additions & 2 deletions diesel/src/types/ord.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use types::{self, NativeSqlType};
use types::{self, NativeSqlType, NotNull};

pub trait SqlOrd {}

Expand All @@ -13,4 +13,4 @@ impl SqlOrd for types::Date {}
impl SqlOrd for types::Interval {}
impl SqlOrd for types::Time {}
impl SqlOrd for types::Timestamp {}
impl<T: SqlOrd + NativeSqlType> SqlOrd for types::Nullable<T> {}
impl<T: SqlOrd + NativeSqlType + NotNull> SqlOrd for types::Nullable<T> {}
3 changes: 2 additions & 1 deletion diesel_codegen/src/associations/belongs_to.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ fn selectable_column_impl(
).unwrap(), quote_item!(cx,
impl ::diesel::expression::SelectableExpression<
::diesel::query_source::LeftOuterJoinSource<$parent_table, $child_table>,
::diesel::types::Nullable<<$column as ::diesel::Expression>::SqlType>,
<<$column as ::diesel::Expression>::SqlType
as ::diesel::types::IntoNullable>::Nullable,
> for $column {}
).unwrap()].to_vec()
}
3 changes: 2 additions & 1 deletion diesel_codegen/src/associations/has_many.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ fn selectable_column_impl(
).unwrap(), quote_item!(cx,
impl ::diesel::expression::SelectableExpression<
::diesel::query_source::LeftOuterJoinSource<$foreign_table, $table>,
::diesel::types::Nullable<<$column as ::diesel::Expression>::SqlType>,
<<$column as ::diesel::Expression>::SqlType
as ::diesel::types::IntoNullable>::Nullable,
> for $column {}
).unwrap()].to_vec()
}

0 comments on commit 29280bf

Please sign in to comment.