mfenniak / rethinkdb-net

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

Atomic update throws InvalidOperationException #246

Closed JoeStead closed 8 years ago

JoeStead commented 8 years ago

When chaining together a GetQuery and UpdateQuery, some horrible expression-based exception is thrown:

Exception thrown: 'System.InvalidOperationException' in System.Core.dll

Additional information: variable 'o' of type 'RethinkSandbox.TestObject' referenced from scope '', but it is not defined

The C# that throws this exception is:

var result = _dbConnection.Run(_table.Get(_currentKey).Update(o => UpdateObject(o)));

Which is based on the REQL:

r.table("foo").get(id).update({hits: r.row("hits") + 1}).run(conn, callback);

Found on the consistency page of the documentation.

It could be by understanding of the query is misplaced and this makes no sense, but it seems a bit suspect that you can't update specific records without first using the Filter method (which makes the operation non-atomic).

mfenniak commented 8 years ago

I believe the problem here is the 'UpdateObject(o)' method call. This can't be executed client-side because 'o' is a reference to a server-side object, and it can't be converted into a server-side expression because the drive can't convert arbitrary functions.

mfenniak commented 8 years ago

Just to clarify, I believe a working version of the JS query you've provided would be:

_dbConnection.Run(_table.Get(_currentKey).Update(o => new WhateverType { hits = o.hits + 1 })```

This is similar to the code in (https://github.com/mfenniak/rethinkdb-net/blob/5853690123ff54f70beb014d7c72fbea515b0d1e/rethinkdb-net-test/Integration/SingleObjectTests.cs#L107), or (https://github.com/mfenniak/rethinkdb-net/blob/5853690123ff54f70beb014d7c72fbea515b0d1e/rethinkdb-net-test/Integration/RealtimePushTests.cs#L185), which are part of the driver unit tests so they pretty definitely work.

JoeStead commented 8 years ago

Yeah, that makes loads of sense. I had a "ugh what" day I think. Thanks for clearing it up :)