Diesel: foo.is_null() is not the same as foo.eq(None)

[icefoxen]

Setup

I have a model something like this:

CREATE TABLE things (
   id INTEGER PRIMARY KEY NOT NULL,
   data INTEGER NOT NULL,
   optional_data VARCHAR
);

pub struct Thing {
    pub id: i32,
    pub data: i32,
    pub optional_data: Option<String>,
}

Versions

• Rust: 1.21
• Diesel: 0.16
• Database: SQLite3 (haven't tried on Postgres)
• Operating System Linux

Problem Description

I want to get all the Things with a particular data value and no optional data, so I wrote:

transactions.filter(                                                                                              
            data.eq(target_data)
                .and(optional_data.eq(None as Option<String>)                                                 
        )

But, it never succeeds even when there is data I would expect to match. Instead, to do this query I have to write:

transactions.filter(                                                                                              
            data.eq(target_data)
                .and(optional_data.is_null())                                                 
        )

It does appear that this is perfectly consistent with SQL (though if there's implementation-defined variations here I don't know anything about it), since SQL NULL is not equal to any value: https://stackoverflow.com/questions/1833949/why-is-null-not-equal-to-null-false#1833989

But, it is extremely inconsistent with Rust and its representation of Option for "missing data". SQL says NULL is never equal to anything including itself, similar to floating point NaN's. But Rust says that an Option<T>::None is equal to any other Option<T>::None. I sort of feel that if you're going to use SQL semantics, you should define a separate type than Option with explicitly different behavior for this, so it doesn't look like it SHOULD obey Rust semantics. Given that optional_data.eq(None) is NEVER true, it seems like it should be impossible to say.

Thank you!

[sgrif]

Making this change would mean that Option<T> no longer generates consistent SQL, which would break our prepared statement caching. Ultimately I don't think it's a bad thing for Diesel to be very close to SQL, which is what we always err towards. Note that we do support the PostgreSQL IS NOT DISTINCT FROM operator with .is_not_distinct_from, which has the semantics you want.

[joelgallant]

For anybody who stumbles on this like I did, it's fairly easy to make a macro that deals with it.

macro_rules! check_nullable {
    ($query:expr, $val:expr, $table_field:expr, $check_null:expr) => {
        match $val {
            // there's a value, no need to check is_null
            Some(ref val) => $query.filter($table_field.eq(val)),
            None => match $check_null {
                // there was no value, and we want to check IS NULL
                true => $query.filter($table_field.is_null()),
                // there was no value, but we won't check IS NULL
                false => $query,
            },
        }
    };

    ($query:expr, $val:expr, $table_field:expr) => {
        check_nullable!($query, $val, $table_field, false)
    };
}

the query needs to be boxed (because of the conditional), but this simplifies

if let Some(ref foo) = self.foo {
    query = query.filter(table::foo.eq(foo));
} else {
    query = query.filter(table::foo.is_null());
}

into

query = check_nullable!(query, self.foo, table::foo, true);

Probably also possible to do with generics, but the trait constraints are quite hairy.

[ensc]

Another implementation:

macro_rules! eq_null {
    ($v:expr,$field:expr) => {
        {|| {
            use crate::diesel::BoolExpressionMethods;
            use crate::diesel::expression::AsExpression;

            sql("").bind::<diesel::sql_types::Bool,_>($v.is_none())
                .and($field.is_null())
                .or(sql("").bind::<diesel::sql_types::Bool,_>(!$v.is_none())
                    .and($field.eq($v)))
        }}()
    };
}

This creates a query fragment like

-- when v is None
WHERE ... ((TRUE AND field IS NULL) OR (FALSE AND field = <value>)) ...

-- when v is Some
WHERE ... ((FALSE AND field IS NULL) OR (TRUE AND field = <value>)) ...

Perhaps there can be added some method implementing this idea?

[SWW13]

Could we at least get a note in the eq / ne documentation for this special case?

It's a bit frustrating when one expects None working like in Rust across the Rust/SQL boundary when in reality it's implicitly converted to NULL which doesn't behave like None in Rust.

[weiznich]

@SWW13 Sure we could document that "special" case, just open a PR doing this.

Generally speaking: From our point of view that behaviour is not surprising because each each part of the expressions just maps literally to sql. Changing that would introduce a wired special case where this straight forward mapping is not true any more. Additionally it would break some performance optimizations around prepared statement cashing for all queries containing a .eq expression and therefore slow down those queries.
That said it is totally possible to implement such an operator as third party crate and publish it to crates.io.

@ensc This already exists, see the method Sean linked above.

[ensc]

@weiznich you mean is_not_distinct_from? afais, this is for postgres only, but not e.g. for sqlite

[icefoxen]

I agree that it makes sense from the SQL side, it's just unexpected from the Rust side. So, documentation is the best fix.

[GopherJ]

I don't think it should be closed, it's a tricky situation...........while the filter is big......

[weiznich]

@GopherJ Could you please elaborate what argument above exactly have changed and why this should be reopened? Otherwise: Why do you post to an old thread if you don't have to add something meaningful to the discussion?

[bazald]

Just came across this myself. That is_null() exists at all feels very odd. The other syntax, which doesn't work apparently, is much more intuitive when writing Rust.

[weiznich]

@bazald Please do not comment to closed issues without adding new information or insights. That is all already discussed and explained multiple times above.