New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

foo.is_null() is not the same as foo.eq(None) #1306

Closed
icefoxen opened this Issue Nov 16, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@icefoxen

icefoxen commented Nov 16, 2017

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!

@icefoxen icefoxen changed the title from fo.is_null() is not the same as foo.eq(None) to foo.is_null() is not the same as foo.eq(None) Nov 16, 2017

@sgrif

This comment has been minimized.

Member

sgrif commented Jan 8, 2018

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.

@sgrif sgrif closed this Jan 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment