You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by davisp <gi...@git.apache.org> on 2016/04/21 22:44:23 UTC
[GitHub] couchdb-couch pull request: Fix couch_key_tree:get_key_leafs/2
GitHub user davisp opened a pull request:
https://github.com/apache/couchdb-couch/pull/167
Fix couch_key_tree:get_key_leafs/2
This is a fix for a long standing bug when retrieving all leaf keys for
a given set of keys. Before this patch we would incorrectly return some
keys as missing if they happened to be deeper on a branch for a key that
was found if they weren't themselves a leaf key.
COUCHDB-2863
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-couch 2863-fix-couch-key-tree-get-key-leafs-2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-couch/pull/167.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #167
----
commit 565eeaa89e32f4d1fb786fe2e98abee422cec8d8
Author: Paul J. Davis <pa...@gmail.com>
Date: 2016-04-21T20:41:01Z
Fix couch_key_tree:get_key_leafs/2
This is a fix for a long standing bug when retrieving all leaf keys for
a given set of keys. Before this patch we would incorrectly return some
keys as missing if they happened to be deeper on a branch for a key that
was found if they weren't themselves a leaf key.
COUCHDB-2863
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Fix couch_key_tree:get_key_leafs/2
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-couch/pull/167
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Fix couch_key_tree:get_key_leafs/2
Posted by nickva <gi...@git.apache.org>.
Github user nickva commented on the pull request:
https://github.com/apache/couchdb-couch/pull/167#issuecomment-213863951
+1
Verified this along with https://github.com/apache/couchdb-couch/pull/166 and https://github.com/apache/couchdb-fabric/pull/47
Don't completely grok the the code, but eunit tests pass, and function_clause error from https://issues.apache.org/jira/browse/COUCHDB-2863 doesn't happen anymore.
```
$ http 'http://adm:pass@localhost:15984/db/doc?open_revs=["1-967a00dff5e02add41819138abb3284d", "2-7051cbe5c8faecd085a3fa619e6e6337"]&latest=true'
HTTP/1.1 200 OK
Content-Type: multipart/mixed; boundary="958cf62657e616ab7d2e799d0d268bf6"
Date: Sun, 24 Apr 2016 01:04:15 GMT
Server: CouchDB/b49d069 (Erlang OTP/17)
Transfer-Encoding: chunked
--958cf62657e616ab7d2e799d0d268bf6
Content-Type: application/json
{"_id":"doc","_rev":"2-7051cbe5c8faecd085a3fa619e6e6337"}
--958cf62657e616ab7d2e799d0d268bf6--
```
Deleting the doc and getting latest works as expected as well:
```
$ http delete http://adm:pass@localhost:15984/db/doc?rev=2-7051cbe5c8faecd085a3fa619e6e6337
{
"id": "doc",
"ok": true,
"rev": "3-7379b9e515b161226c6559d90c4dc49f"
}
$ http 'http://localhost:15984/db/doc?open_revs=["1-967a00dff5e02add41819138abb3284d", "2-7051cbe5c8faecd085a3fa619e6e6337"]&latest=true'
HTTP/1.1 200 OK
Content-Type: multipart/mixed; boundary="85b637594584dc5bb88940a5513ebc74"
Date: Sun, 24 Apr 2016 01:32:20 GMT
Server: CouchDB/b49d069 (Erlang OTP/17)
Transfer-Encoding: chunked
--85b637594584dc5bb88940a5513ebc74
Content-Type: application/json
{"_id":"doc","_rev":"3-7379b9e515b161226c6559d90c4dc49f","_deleted":true}
--85b637594584dc5bb88940a5513ebc74--
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Fix couch_key_tree:get_key_leafs/2
Posted by davisp <gi...@git.apache.org>.
Github user davisp commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/167#discussion_r63379218
--- Diff: src/couch_key_tree.erl ---
@@ -280,25 +280,41 @@ get_key_leafs([{Pos, Tree}|Rest], Keys, Acc) ->
{Gotten, RemainingKeys} = get_key_leafs_simple(Pos, [Tree], Keys, []),
get_key_leafs(Rest, RemainingKeys, Gotten ++ Acc).
-get_key_leafs_simple(_Pos, _Tree, [], _KeyPathAcc) ->
+get_key_leafs_simple(_Pos, _Tree, [], _PathAcc) ->
{[], []};
-get_key_leafs_simple(_Pos, [], KeysToGet, _KeyPathAcc) ->
- {[], KeysToGet};
-get_key_leafs_simple(Pos, [{Key, _Value, SubTree}=Tree | RestTree], KeysToGet, KeyPathAcc) ->
- case lists:delete({Pos, Key}, KeysToGet) of
- KeysToGet -> % same list, key not found
- {LeafsFound, KeysToGet2} = get_key_leafs_simple(Pos + 1, SubTree, KeysToGet, [Key | KeyPathAcc]),
- {RestLeafsFound, KeysRemaining} = get_key_leafs_simple(Pos, RestTree, KeysToGet2, KeyPathAcc),
- {LeafsFound ++ RestLeafsFound, KeysRemaining};
- KeysToGet2 ->
- LeafsFound = get_all_leafs_simple(Pos, [Tree], KeyPathAcc),
- LeafKeysFound = [{LeafPos, LeafRev} || {_, {LeafPos, [LeafRev|_]}}
- <- LeafsFound],
- KeysToGet3 = KeysToGet2 -- LeafKeysFound,
- {RestLeafsFound, KeysRemaining} = get_key_leafs_simple(Pos, RestTree, KeysToGet3, KeyPathAcc),
- {LeafsFound ++ RestLeafsFound, KeysRemaining}
+get_key_leafs_simple(_Pos, [], Keys, _PathAcc) ->
+ {[], Keys};
+get_key_leafs_simple(Pos, [{Key, _, SubTree}=Tree | RestTree], Keys, PathAcc) ->
+ NewPathAcc = [Key | PathAcc],
--- End diff --
Good catch. That should be moved down into the first clause. It was just a tidy for line length. I'm also going to add a couple comments to get_key_leafs_simple2 because that took me entirely too long to page back in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Fix couch_key_tree:get_key_leafs/2
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch/pull/167#issuecomment-219471490
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: Fix couch_key_tree:get_key_leafs/2
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/167#discussion_r63335812
--- Diff: src/couch_key_tree.erl ---
@@ -280,25 +280,41 @@ get_key_leafs([{Pos, Tree}|Rest], Keys, Acc) ->
{Gotten, RemainingKeys} = get_key_leafs_simple(Pos, [Tree], Keys, []),
get_key_leafs(Rest, RemainingKeys, Gotten ++ Acc).
-get_key_leafs_simple(_Pos, _Tree, [], _KeyPathAcc) ->
+get_key_leafs_simple(_Pos, _Tree, [], _PathAcc) ->
{[], []};
-get_key_leafs_simple(_Pos, [], KeysToGet, _KeyPathAcc) ->
- {[], KeysToGet};
-get_key_leafs_simple(Pos, [{Key, _Value, SubTree}=Tree | RestTree], KeysToGet, KeyPathAcc) ->
- case lists:delete({Pos, Key}, KeysToGet) of
- KeysToGet -> % same list, key not found
- {LeafsFound, KeysToGet2} = get_key_leafs_simple(Pos + 1, SubTree, KeysToGet, [Key | KeyPathAcc]),
- {RestLeafsFound, KeysRemaining} = get_key_leafs_simple(Pos, RestTree, KeysToGet2, KeyPathAcc),
- {LeafsFound ++ RestLeafsFound, KeysRemaining};
- KeysToGet2 ->
- LeafsFound = get_all_leafs_simple(Pos, [Tree], KeyPathAcc),
- LeafKeysFound = [{LeafPos, LeafRev} || {_, {LeafPos, [LeafRev|_]}}
- <- LeafsFound],
- KeysToGet3 = KeysToGet2 -- LeafKeysFound,
- {RestLeafsFound, KeysRemaining} = get_key_leafs_simple(Pos, RestTree, KeysToGet3, KeyPathAcc),
- {LeafsFound ++ RestLeafsFound, KeysRemaining}
+get_key_leafs_simple(_Pos, [], Keys, _PathAcc) ->
+ {[], Keys};
+get_key_leafs_simple(Pos, [{Key, _, SubTree}=Tree | RestTree], Keys, PathAcc) ->
+ NewPathAcc = [Key | PathAcc],
--- End diff --
I'm not clear why you pulled this up when it's only used once. Is this is a tidy only or did you mean to use it at 298?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---