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

Inconsistent NULL behavior #21

Closed
zmitchell opened this Issue Dec 26, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@zmitchell

zmitchell commented Dec 26, 2017

Say that you create a model like this:

#[derive(SqlTable)]
struct MyModel {
    id: PrimaryKey,
    foo: Option<String>,
}

Now try to insert a value that has None in the foo field.

let id = sql!(MyModel.insert(foo = None));

You'll get a type error saying that foo should be of type String. If you want to disallow NULL fields, you should also disallow creating a model containing Option<T>.

@antoyo

This comment has been minimized.

Owner

antoyo commented Dec 30, 2017

Oh, actually, you just need to avoid specifiying the value, but I guess this might not always be what we want, i.e. when we have a variable of type Option<String>.
So, the following code works:

let id = sql!(MyModel.insert());

So, I guess accepting both the types String and Option<String> would fix this issue.
Thanks for reporting this issue.

@antoyo antoyo added bug easy labels Dec 31, 2017

@zmitchell

This comment has been minimized.

zmitchell commented Jan 1, 2018

I'm not sure I follow. What do you mean "you just need to avoid specifying the value"? It also fails when you try to insert a record with more than one field (with at least one of them being Option<T>).

@antoyo

This comment has been minimized.

Owner

antoyo commented Jan 1, 2018

The current way to set a value to NULL is to not specify it in the insert() method call:

let id = sql!(MyModel.insert(/* the value is not here*/);

If you want to insert a value instead of NULL, specify it without Some.

let id = sql!(MyModel.insert(foo = "string");

Just try the code I posted previously using the model from your first post and it should work.
Can you show me an example for your second point?
Thank you.

@antoyo antoyo referenced this issue Jan 12, 2018

Merged

Fix/design issue 16 #23

3 of 3 tasks complete
@antoyo

This comment has been minimized.

Owner

antoyo commented Jan 21, 2018

Fixed by #23 and tested in #27.

@antoyo antoyo closed this Jan 21, 2018

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