Closed ckmaresca closed 10 years ago
Hi @ckmaresca, the error message you are seeing actually comes from the RethinkDB server, not from the driver itself. You are probably just running into the same issue described in this comment by robert-zaremba: https://github.com/rethinkdb/rethinkdb/issues/1570#issuecomment-31987685 A new version of the RethinkDB server 1.11.3 will be released in the next days, and will fix this problem.
I'm keeping this open until we can verify that RethinkDB 1.11.3 fixes this.
Hmmm, strangely enough I ran to the exact same error in 1.7.x and upgraded to 1.11.x as I thought it might have been due to old protobuffer implementations....
Thanks for the update. It's not great news as I'm already weeks late on this project and was hoping to finish the last few things, but this is a hard stop, unfortunately.
@ckmaresca You can work around it by trying an insert first. If the insert fails because the document doesn't exist, you would try the replace. Only problem would be if the document is deleted between the insert and the replace. Then the replace would fail too. You could run the insert and replace in a loop if that's an option.
I've thought of that and I already have some code that does something similar. The problem is that this 'append' is a callback for several (upto 12) different background processes, basically a data aggregation. You can see what kinds of data collisions might happen if I did what you mentioned. The other solution is to write the results to X separate tables and when all the background tasks have completed, merge, then insert the results. Not pretty and way more work than I'd like.
Of course, the real solution is to have an '$push' statement similar to Mongo's (or just use Mongo instead...), see http://docs.mongodb.org/manual/reference/operator/update/push/
It's not often I run into a showstopper bug, but this is one of them. And I can't remember the last time it happened when I was 98% done writing something... Ah, well, such is life.
@ckmaresca Well, ReQL update
could do the same thing as Mongo's $push
, if this bug just was fixed in the server.
Do your background processes ever delete the rows? The "collision" in the mentioned work-around only happens if rows are deleted. Otherwise it should be guaranteed to work.
There's no guarantee as to what might happen. That's because I'm exposing Rethink through a generic JSON interface that's used by a lot of other processes in a similar fashion to a service bus. So there could be a delete, although there isn't one in the current set of 'clients'.
I think that Mongo's push is a better solution to this sort of thing it's simpler and you don't have to worry about namespace issues. It's also much more straight forward from a readability/maintainability point of view and it's language agnostic, so the same syntax is used everywhere.
Actually, the work-around even seems to be correct if somebody else deletes it between the insert and the replace. Both the insert and replace would fail, but the state of the database will be the same as if the modification succeeded and then the delete happened immediately afterwards. So doing an insert, followed up by replace in case the insert fails, is exactly equivalent to the original query as far as I can see.
Anyways, you can wait for RethinkDB 1.11.3 which will fix this.
@ckmaresca RethinkDB 1.11.3 has been released https://github.com/rethinkdb/rethinkdb/releases/tag/v1.11.3 . The original query using just replace
should now work as expected. Please let me know if the issue is still there. Hope you can still make the project deadline.
Seems to have worked, at least my simple test works. Re: deadline - I'm already weeks late, but, given that I'm one of the founders of the company, I think I might be able to get away with it. I'm just blocking our ability to make any money... ;-)
@ckmaresca So sorry you had to run into this at a critical phase of your project. Hope things with RethinkDB work out better for you from now on.
I'm closing this issue, because it is not a driver bug.
On further tests, it seems like the above function is not merging, it's simply replacing the existing record. It was a little harder for me to test as I'm doing a bunch of asynchronous callbacks, but it doesn't work as expected. Looks like I'm going to have to implement the work around after all...
Edit: Interestingly enough, I attempted to merge a sub-array with the existing record and just replaced the sub-array. So it's not doing a merge with the existing data.
{
"id": "foobar",
"meta": {
"value1": "value1",
"value2": "value2"
}
"results": {
"value3": "value3",
"value3": "value3",
}
}
Attempting to merge "results":
with another array results in replacement, not merging. merge(array('results' => $newresults))
doesn't merge, it replaces
Hmm that is very odd. The merge
in there should definitely make it merge.
Can you run the following script to test this? It worked in my case.
<?php
require("rdb/rdb.php");
$conn = r\connect("localhost");
r\tableCreate("test1")->run($conn);
r\table("test1")->get("a")->replace(function($doc) {
return $doc->rDefault(array('id' => "a"))->merge(array('first' => "first"));
})->run($conn);
echo "Added field first: " . r\table("test1")->get("a")->run($conn) . "\n";
r\table("test1")->get("a")->replace(function($doc) {
return $doc->rDefault(array('id' => "a"))->merge(array('second' => "second"));
})->run($conn);
echo "Added field second: " . r\table("test1")->get("a")->run($conn) . "\n";
r\tableDrop("test1")->run($conn);
$conn->close();
?>
The output should be:
Added field first: array('first' => 'first', 'id' => 'a')
Added field second: array('first' => 'first', 'id' => 'a', 'second' => 'second')
Or wait, given that you mentioned Mongo's $push, did you actually want to append a new value to some array in the document?
In that case merge
is not the right ReQL term. Use append
http://php-rql.dnsalias.net/api/#ph:document_manipulation-append !
For example if you want to append the new value $request to the field 'manyValues', leaving any existing fields in the document intact and creating the document if it doesn't already exist:
r\table($table)->get($id)->replace(
function($doc) use ($id,$request) {
return $doc->rDefault(array('id' => $id))->merge(array('manyValues' => $doc('manyValues')->rDefault(array())->append($request)));
}
)->run($conn);
(You can simplify that a little if you can make sure that the field 'manyValues' always exists in documents in this table.)
I'm looking at your code and thinking that my data structures are far more complicated (see above). Could you try it with nested arrays, something like:
$foo = array(
'this' => array(
array('one' => 'one','two' => 'two'),
array('three' => 'three','four'=>'four')
),
'that' => array(
array('five' => 'five','six' => 'six'),
array('seven' => 'seven','eight'=>'eight')
)
}
merge with
$bar = array(
'this1' => array(
array('one1' => 'one1','two1' => 'two1'),
array('three1' => 'three1','four1'=>'four1')
),
'that1' => array(
array('five1' => 'five1','six1' => 'six1'),
array('seven1' => 'seven1','eight1'=>'eight1')
)
}
Ugh, cross commenting - yes, append is what I'm looking for. I asked for this in the Google Group & Lucy came up with merge
. I did look up append
but he didn't go that route, so I followed blindly...
Edit: I try your code in just a sec. Hopefully this works...
Hmm, append seems to work, but creating a new document when there is no record is not working, throws an error: Expected type ARRAY but found OBJECT.
If you go back to this comment: https://github.com/danielmewes/php-rql/issues/52#issuecomment-32632169 - that's basically my data structure (although under results
, there are a bunch of arrays instead of strings). A new document looks pretty much like that whole JSON structure, but the append only occurs under results
, the other parts don't get touched. And results
should always exist.
Funny enough it took more time to write this than to test...
Oh could it be that you are using an older version of PHP-RQL? Those converted an empty array()
into an empty OBJECT instead of an empty ARRAY. This behavior was changed in PHP-RQL 1.8.0, and array()
is now converted into an ARRAY so this should work (see the "Automatic Conversion" part on http://php-rql.dnsalias.net/wiki/index.php/RqlDataModel ).
Instead of
...->merge(array('results' => $doc('results')->rDefault(array())->append($request))
you can use
...->merge(array('results' => $doc('results')->rDefault(new r\ArrayDatum(array()))->append($request))
which should work with any version of PHP-RQL.
I don't know, how would I check the version? I'm looking in the dir now, and there's no clear indication of version.... Can I update it by simply overwriting the existing dir?
@ckmaresca Unfortunately there isn't really a good way for finding out which version you are running (apart from comparing the source files). I opened https://github.com/danielmewes/php-rql/issues/53 for this.
Are you using Composer or downloaded PHP-RQL manually? If you downloaded it manually just download the new version from http://danielmewes.github.io/php-rql/ and replace it (http://php-rql.dnsalias.net/wiki/index.php/Introduction). In case of Composer I actually don't know how that works. You could just pin it to a specific version instead of using the "dev-master" version (which is always the most recent release, but probably Composer doesn't update it automatically?). 1.11.1 is the most recent one. See https://packagist.org/packages/danielmewes/php-rql
I just updated it by downloading manually and move src/rdb
. Looks like you changed the dir structure over to composer (not sure when), but it was easier for me to do it this way. It no longer throws an error, so that's resolved.
Still struggling with getting php-rql to do what I want, although it does create a new document now, it's just the wrong structure. The basic logic I'm looking for is this:
$id
existsif
no create complete new document from $request
(see structure above) with id => $id
if
yes append records from $request['results']
to sub-array results
Right now what is happening is that new document creation basically creates a new document but only with results
not the rest of the data.... Append does work as expected.
Here's my actual code:
r\table($table)->get($id)->replace(
function($doc) use($id,$request) {
return $doc
->rDefault(array('id' => $id))
->merge(array('results' => $doc('results')
->rDefault(array())
->append($request['results'])));
}
)->run($conn);
Obviously there's no reference to the full $request
, only to $request['results']
, but I'm not really sure how to translate my pseudo code into something that works in RQL...
PS - I'm pretty sure you just do composer update
to update composer
packages, but I rarely do it as I don't like to change things unless I absolutely have to.
Note: code edited for legitability
Something like this might be what you want:
$defaultDocument = array(
"id" => $id,
"meta" => array(
"value1": "default value 1",
"value2": "default value 2"),
"results" => array()
);
r\table($table)->get($id)->replace(
function($doc) use($defaultDocument, $request) {
return $doc
->rDefault($defaultDocument)
->merge(array('results' => $doc('results')->append($request['results'])));
}
)->run($conn);
Oh so one more thing: append
is for appending a single element. So if $request['results']
is itself an array containing multiple elements, you have to use spliceAt
(http://php-rql.dnsalias.net/api/#ph:document_manipulation-splice_at) like here:
$defaultDocument = ...;
r\table($table)->get($id)->replace(
function($doc) use($defaultDocument, $request) {
return $doc
->rDefault($defaultDocument)
->merge(array('results' => r\expr($request['results'])->spliceAt(0, $doc('results'))));
}
)->run($conn);
Actually, append
works fine for arrays, at least it does the few times I've run it... Thx for all the pointers, this is definitely non-trivial and I could not even imagine getting this far without help.
Hhmm, I tried both variations and I'm getting the following error Cannot perform get_field on a non-object non-sequence``null``.
I don't know if it makes any difference, but in my case $defaultDocument == $request
- I've replaced $defaultDocument
in both references with $request
.
Oops my mistake. The problem is that we are using $doc
in the merge(...)
, which is the original document and not the one that has rDefault
applied to it.
Make that:
$defaultDocument = ...;
r\table($table)->get($id)->replace(
r\row()->rDefault($defaultDocument)->rDo(
function($doc) use($request) {
return $doc
->merge(array('results' => r\expr($request['results'])->spliceAt(0, $doc('results'))));
}
)
)->run($conn);
This really is becoming more complicated than I had thought...
That is one complicated query. Def would have never figured that out. Now lets see if it works... ;-)
Hhhmmm.
Type: r\RqlUserError
Message:Compile error: r.row is not defined in this context.
P.S. Had to LOL about the complexity. I mean, it seems like such an easy problem - append data to an existing sub-array.... Basically just $array['foobar'][] = array('foo' => 'bar')
Like I said before, it seems like this should be something that's handled with a native query. Although I've referred Mongo's append $post
, I wonder if it has similar issues. Certainly a feature like this could be a key feature that makes Rethink different than other JSON nosql db's...
Oh no. Now that one might actually be an oversight in PHP-RQL. I tested it in JavaScript and there I could use r.row, but apparently not here.
New version without r\row():
$defaultDocument = ...;
r\table($table)->get($id)->replace(
function ($docOrNull) use ($defaultDocument, $request) {
return $docOrNull->rDefault($defaultDocument)->rDo(
function($doc) use($request) {
return $doc
->merge(array('results' => r\expr($request['results'])->spliceAt(0, $doc('results'))));
}
);
}
)->run($conn);
Don't want to jinx it, but looks like that worked on both insert & update. At least it did when I sent raw data, now I need to test it as part of the whole codebase.
Thanks so much for spending so much time on this. There's no way I could have figured this out, that final query is insane with pretty much every trick in the book. I would definitely stick this one in the FAQ/cookbook/wiki as there is lots of good stuff going on here.
P.S. I've run into r\row missing in the past. There's much more documentation on how to do things in JS, but a lot of it relies on r\row.
Just to close this out, I just tried this in my wider app, looks like it works. Thanks again for the help.
I opened an issue in the RethinkDB issue tracker to discuss possible changes that could make this easier in the future. https://github.com/rethinkdb/rethinkdb/issues/1877
Possibly the final answer on how to handle this ( from https://github.com/rethinkdb/rethinkdb/issues/1877 ) using branch
and add
r\table($table)->get($id)->replace( function($doc) use ($request) {
return r\branch(
$doc->eq(null),
$request,
$doc->merge(array('results' => r\expr($request['results'])->add($doc('results'))))
);
}
)->run($conn);
Quite a cool solution for automatically creating a new record/document, then appending to a sub-array in subsequent updates.
The following code seems to fail with:
Runtime error: Expected type OBJECT but found NULL.
Judging from the stack trace below, it seems to be because of the check in the return function. I'm unsure on how to debug this further.
Any ideas how to make this work? I'm stuck until I can find a work around for this... I would note that both $id and $replace are in the scope of the function and that replacing them with static values throws the same error...