You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by "Jan Lehnardt (JIRA)" <ji...@apache.org> on 2011/04/17 14:20:05 UTC

[jira] [Commented] (COUCHDB-954) typo in key tree logic when latest=true

    [ https://issues.apache.org/jira/browse/COUCHDB-954?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13020783#comment-13020783 ] 

Jan Lehnardt commented on COUCHDB-954:
--------------------------------------

It's not easy to find a case here that breaks this.

I had to go into etap and provide (as far as I can tell) invalid keys to trigger this behaviour.

This code is frequently exercised, but either KeysToGet2 is [] or LeafKeysFound is not to be found in KeysToGet2.

Or in Erlang terms:

[] = [] -- ["foo"], % no badmatch
["bar"] = ["bar"] -- ["foo"], ; no badmatch

This is the etap test and code fix that exercises the badmatch, but I don't know if we can ever trigger this from the outside:

diff --git a/test/etap/063-kt-get-leaves.t b/test/etap/063-kt-get-leaves.t
index 6d4e800..0de7347 100755
--- a/test/etap/063-kt-get-leaves.t
+++ b/test/etap/063-kt-get-leaves.t
@@ -60,6 +60,12 @@ test() ->
     ),
 
     etap:is(
+        {[{"bar",{1,["1b","1"]}}],[]},
+        couch_key_tree:get_key_leafs(TwoChildSibs, ["bar", {1, "1b"}]),
+        "COUCHDB-954 — invalid keys."
+    ),
+
+    etap:is(
         {[{0,[{"1", "foo"}]}],[]},
         couch_key_tree:get_full_key_paths(TwoChildSibs, [{0, "1"}]),
         "retrieve full key paths."



diff --git a/src/couchdb/couch_key_tree.erl b/src/couchdb/couch_key_tree.erl
index 990f3f2..2cbb45e 100644
--- a/src/couchdb/couch_key_tree.erl
+++ b/src/couchdb/couch_key_tree.erl
@@ -231,8 +231,8 @@ get_key_leafs_simple(Pos, [{Key, _Value, SubTree}=Tree | RestTree], KeysToGet, K
     KeysToGet2 ->
         LeafsFound = get_all_leafs_simple(Pos, [Tree], KeyPathAcc),
         LeafKeysFound = [LeafKeyFound || {LeafKeyFound, _} <- LeafsFound],
-        KeysToGet2 = KeysToGet2 -- LeafKeysFound,
-        {RestLeafsFound, KeysRemaining} = get_key_leafs_simple(Pos, RestTree, KeysToGet2, KeyPathAcc),
+        KeysToGet3 = KeysToGet2 -- LeafKeysFound,
+        {RestLeafsFound, KeysRemaining} = get_key_leafs_simple(Pos, RestTree, KeysToGet3, KeyPathAcc),
         {LeafsFound ++ RestLeafsFound, KeysRemaining}
     end.


> typo in key tree logic when latest=true
> ---------------------------------------
>
>                 Key: COUCHDB-954
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-954
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Database Core
>            Reporter: Adam Kocoloski
>
> This line
> https://github.com/apache/couchdb/blob/7fe84eba9982ebb3bcaa48b7aa28fdd2e130422d/src/couchdb/couch_key_tree.erl#L195
> seems like it should read KeysToGet3 = KeysToGet2 -- LeafKeysFound, and then KeysToGet3 should be supplied to the following invocation of get_key_leafs_simple.  I think the current code would crash if a user made an open_revs call with latest=true and a pair of revisions that have a parent/child relationship, or at least it would crash if latest=true was implemented (COUCHDB-953).

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira