You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by "Adam Kocoloski (JIRA)" <ji...@apache.org> on 2010/11/18 05:32:13 UTC
[jira] Created: (COUCHDB-954) typo in key tree logic when
latest=true
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.
-
You can reply to this email to add a comment to the issue online.
[jira] [Resolved] (COUCHDB-954) typo in key tree logic when
latest=true
Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/COUCHDB-954?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adam Kocoloski resolved COUCHDB-954.
------------------------------------
Resolution: Fixed
Fix Version/s: 1.2
Thanks for the review. I've committed the fix and a slightly expanded JS test to trunk. I didn't get around to incorporating the etap test in this ticket, but I think the JS tests are more illustrative of the feature.
I'm inclined to not backport this patch, since replication works correctly without the optimization and without backporting COUCHDB-953 it won't have any effect at the HTTP layer anyway.
> 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
> Assignee: Adam Kocoloski
> Fix For: 1.2
>
>
> 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
[jira] [Commented] (COUCHDB-954) typo in key tree logic when
latest=true
Posted by "Paul Joseph Davis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/COUCHDB-954?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13091325#comment-13091325 ]
Paul Joseph Davis commented on COUCHDB-954:
-------------------------------------------
Looks good to me.
> 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
> Assignee: 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
[jira] [Commented] (COUCHDB-954) typo in key tree logic when
latest=true
Posted by "Jan Lehnardt (JIRA)" <ji...@apache.org>.
[ 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
[jira] [Commented] (COUCHDB-954) typo in key tree logic when
latest=true
Posted by "Randall Leeds (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/COUCHDB-954?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13091328#comment-13091328 ]
Randall Leeds commented on COUCHDB-954:
---------------------------------------
Looks good from here, too.
> 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
> Assignee: 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
[jira] [Assigned] (COUCHDB-954) typo in key tree logic when
latest=true
Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/COUCHDB-954?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adam Kocoloski reassigned COUCHDB-954:
--------------------------------------
Assignee: Adam Kocoloski
> 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
> Assignee: 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
[jira] [Commented] (COUCHDB-954) typo in key tree logic when
latest=true
Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/COUCHDB-954?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13091321#comment-13091321 ]
Adam Kocoloski commented on COUCHDB-954:
----------------------------------------
Ok, so this was weirder than I expected. We had the format of LeafsFound all mixed up, so we were guaranteed never to match anything in the '--' call.
I posted a patch and a JS test on GitHub:
https://github.com/kocolosk/couchdb/compare/trunk...954-key-tree-latest-true
Didn't get around to writing proper commit messages yet :). I'll take a closer look at Jan's etap and see if we can add that too. But in the meantime would appreciate a review here.
> 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
> Assignee: 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
[jira] [Commented] (COUCHDB-954) typo in key tree logic when
latest=true
Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/COUCHDB-954?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13091255#comment-13091255 ]
Adam Kocoloski commented on COUCHDB-954:
----------------------------------------
Now that latest=true works I really probably should fix this one. Confirmed that Jan's test exercises the code path, will try to write one with real client operations too.
> 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
> Assignee: 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
[jira] [Closed] (COUCHDB-954) typo in key tree logic when
latest=true
Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/COUCHDB-954?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adam Kocoloski closed COUCHDB-954.
----------------------------------
> 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
> Assignee: Adam Kocoloski
> Fix For: 1.2
>
>
> 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
[jira] [Commented] (COUCHDB-954) typo in key tree logic when
latest=true
Posted by "Paul Joseph Davis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/COUCHDB-954?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13091534#comment-13091534 ]
Paul Joseph Davis commented on COUCHDB-954:
-------------------------------------------
+1 on not backporting. Patch looks simple enough, but why chance key_tree changes to a release branch.
> 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
> Assignee: Adam Kocoloski
> Fix For: 1.2
>
>
> 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