mfenniak / rethinkdb-net

A C# / .NET client driver for RethinkDB.
Other
247 stars 37 forks source link

Comparison to null in Filter expression #203

Closed jonorossi closed 9 years ago

jonorossi commented 9 years ago

At first I thought rethinkdb-net didn't support Nullable<DateTime> inside a Filter() expression, so I tried both x == null and x.HasValue, both didn't work, then I tried with a string and that also didn't work. Following is the ReQL query and the C# I'm trying to use, I just get back an empty enumerable without anything matching:

ReQL:

r.db('mydb').table('mytable')
  .filter({my_string: null})

C#:

Query.Db("mydb").Table<MyClass>("mytable")
    .Filter(r => r.my_string == null)
);

I had a look through the source code to work out how I'd fix this, but I'm quite lost and I suspect this fix is going to be quite involved. Any chance you've got some time to fix it or point me in the right direction? Thanks.

mfenniak commented 9 years ago

I've written some unit tests in a branch, PR #204. Oddly, I can't get the behavior that you see where you get back an empty enumerable. I created three test cases with two variations:

I've only had errors where the code won't work at all. None of them worked but caused no results to return. Would you mind taking a look at my tests and see if they represent the situation you're running into?

jonorossi commented 9 years ago

I'm not sure what Nancy is doing, but just throwing back the RethinkDb.SynchronousApiExtensions+AsyncEnumerableSynchronizer<MyModel> must be swallowing the exception. Explicitly calling ToList() throws a System.NotSupportedException: Datum converter is not available for type System.Object, which I remember seeing when I was building up a bigger query outside of the Nancy module which I couldn't get to compile.

Many thanks for looking at this so quickly, sorry for sending you on a wild goose chase.

mfenniak commented 9 years ago

Great; the System.NotSupportedException: Datum converter is not available for type System.Object error is the same that I've got for the third test case. I'll have that fixed shortly.

mfenniak commented 9 years ago

I believe this issue is fixed in the issue_203 branch. If you have the time, could you take a look and verify before I merge that branch? :-)

jonorossi commented 9 years ago

Many thanks Mathieu. I just tested your changes and they work for me.

I can see the fix is not quite obvious, but I'm getting a better idea of how this library and rethinkdb communicate.

mfenniak commented 9 years ago

Thanks for the issue report, and the other PRs you submitted. I love having new contributors to this project. :-) Let me know if there's anything I can do to help you understand and participate in the project more.