You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2020/08/05 18:28:15 UTC
[GitHub] [couchdb] davisp opened a new pull request #3058: Ebtree fixes
davisp opened a new pull request #3058:
URL: https://github.com/apache/couchdb/pull/3058
Fixes for ebtree
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on a change in pull request #3058: Ebtree fixes
Posted by GitBox <gi...@apache.org>.
rnewson commented on a change in pull request #3058:
URL: https://github.com/apache/couchdb/pull/3058#discussion_r465954608
##########
File path: src/ebtree/src/ebtree.erl
##########
@@ -282,6 +283,9 @@ reduce(Db, #tree{} = Tree, StartKey, EndKey, Options) ->
do_reduce(Tree, MapValues, ReduceValues).
+do_reduce(#tree{} = Tree, [], []) ->
+ reduce_values(Tree, [], false);
Review comment:
kk
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] davisp merged pull request #3058: Ebtree fixes
Posted by GitBox <gi...@apache.org>.
davisp merged pull request #3058:
URL: https://github.com/apache/couchdb/pull/3058
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on a change in pull request #3058: Ebtree fixes
Posted by GitBox <gi...@apache.org>.
rnewson commented on a change in pull request #3058:
URL: https://github.com/apache/couchdb/pull/3058#discussion_r465938416
##########
File path: src/ebtree/src/ebtree.erl
##########
@@ -411,6 +419,9 @@ range(Db, #tree{} = Tree, StartKey, EndKey, AccFun, Acc0) ->
end).
+range(_Tx, #tree{}, #node{level = 0, members = []}, _StartKey, _EndKey, _AccFun, Acc0) ->
Review comment:
got it. can you add a test that shows the problem pls.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on a change in pull request #3058: Ebtree fixes
Posted by GitBox <gi...@apache.org>.
rnewson commented on a change in pull request #3058:
URL: https://github.com/apache/couchdb/pull/3058#discussion_r465957927
##########
File path: src/ebtree/src/ebtree.erl
##########
@@ -411,6 +419,9 @@ range(Db, #tree{} = Tree, StartKey, EndKey, AccFun, Acc0) ->
end).
+range(_Tx, #tree{}, #node{level = 0, members = []}, _StartKey, _EndKey, _AccFun, Acc0) ->
Review comment:
also tighten this to match only the root node (instead of `level=0`)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] rnewson commented on a change in pull request #3058: Ebtree fixes
Posted by GitBox <gi...@apache.org>.
rnewson commented on a change in pull request #3058:
URL: https://github.com/apache/couchdb/pull/3058#discussion_r465926426
##########
File path: src/ebtree/src/ebtree.erl
##########
@@ -411,6 +419,9 @@ range(Db, #tree{} = Tree, StartKey, EndKey, AccFun, Acc0) ->
end).
+range(_Tx, #tree{}, #node{level = 0, members = []}, _StartKey, _EndKey, _AccFun, Acc0) ->
Review comment:
when will a leaf node have zero members? That... should never happen.
##########
File path: src/ebtree/src/ebtree.erl
##########
@@ -282,6 +283,9 @@ reduce(Db, #tree{} = Tree, StartKey, EndKey, Options) ->
do_reduce(Tree, MapValues, ReduceValues).
+do_reduce(#tree{} = Tree, [], []) ->
+ reduce_values(Tree, [], false);
Review comment:
shouldn't that be `true`? the lack of map values is the signal we're working with reductions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] davisp commented on a change in pull request #3058: Ebtree fixes
Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #3058:
URL: https://github.com/apache/couchdb/pull/3058#discussion_r465927945
##########
File path: src/ebtree/src/ebtree.erl
##########
@@ -411,6 +419,9 @@ range(Db, #tree{} = Tree, StartKey, EndKey, AccFun, Acc0) ->
end).
+range(_Tx, #tree{}, #node{level = 0, members = []}, _StartKey, _EndKey, _AccFun, Acc0) ->
Review comment:
When the tree is empty like it says in the commit message.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] davisp commented on a change in pull request #3058: Ebtree fixes
Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #3058:
URL: https://github.com/apache/couchdb/pull/3058#discussion_r465930718
##########
File path: src/ebtree/src/ebtree.erl
##########
@@ -282,6 +283,9 @@ reduce(Db, #tree{} = Tree, StartKey, EndKey, Options) ->
do_reduce(Tree, MapValues, ReduceValues).
+do_reduce(#tree{} = Tree, [], []) ->
+ reduce_values(Tree, [], false);
Review comment:
We're not working with reductions though. We're reducing an empty list of key/value pairs. Plainly the empty list from the second argument is the same empty list being passed to reduce_values.
Also, I think most folks writing reduce functions would do better to think of it as "reduce empty list of key/values" vs "rereduce empty list of reduce values that might need to be guarded against for the normal case when you actually have reduce values".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org