You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by da...@apache.org on 2017/04/06 16:16:25 UTC
[11/36] couch commit: updated
refs/heads/COUCHDB-3287-pluggable-storage-engines to 0fbcd0c
Return error row instead of crashing
When input is invalid for built in reducers, we return an error row
instead of crashing. Thanks to davisp for providing the elegant
solution.
COUCHDB-3305
Project: http://git-wip-us.apache.org/repos/asf/couchdb-couch/repo
Commit: http://git-wip-us.apache.org/repos/asf/couchdb-couch/commit/9f9c4822
Tree: http://git-wip-us.apache.org/repos/asf/couchdb-couch/tree/9f9c4822
Diff: http://git-wip-us.apache.org/repos/asf/couchdb-couch/diff/9f9c4822
Branch: refs/heads/COUCHDB-3287-pluggable-storage-engines
Commit: 9f9c48221a96452f36790aa8dd52c557d65b9cde
Parents: 9d8be06
Author: Tony Sun <to...@cloudant.com>
Authored: Tue Feb 21 14:18:49 2017 -0800
Committer: Tony Sun <to...@cloudant.com>
Committed: Tue Mar 7 19:31:20 2017 -0800
----------------------------------------------------------------------
src/couch_query_servers.erl | 66 ++++++++++++++++++++++++++++++++++------
1 file changed, 56 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/couchdb-couch/blob/9f9c4822/src/couch_query_servers.erl
----------------------------------------------------------------------
diff --git a/src/couch_query_servers.erl b/src/couch_query_servers.erl
index 92d9e24..ddc079e 100644
--- a/src/couch_query_servers.erl
+++ b/src/couch_query_servers.erl
@@ -27,7 +27,7 @@
-include_lib("couch/include/couch_db.hrl").
-define(SUMERROR, <<"The _sum function requires that map values be numbers, "
- "arrays of numbers, or objects, not '~p'. Objects cannot be mixed with other "
+ "arrays of numbers, or objects. Objects cannot be mixed with other "
"data structures. Objects can be arbitrarily nested, provided that the values "
"for all fields are themselves numbers, arrays of numbers, or objects.">>).
@@ -142,25 +142,34 @@ os_rereduce(Lang, OsRedSrcs, KVs) ->
builtin_reduce(_Re, [], _KVs, Acc) ->
{ok, lists:reverse(Acc)};
builtin_reduce(Re, [<<"_sum",_/binary>>|BuiltinReds], KVs, Acc) ->
- Sum = builtin_sum_rows(KVs),
+ Sum = builtin_sum_rows(KVs, []),
builtin_reduce(Re, BuiltinReds, KVs, [Sum|Acc]);
builtin_reduce(reduce, [<<"_count",_/binary>>|BuiltinReds], KVs, Acc) ->
Count = length(KVs),
builtin_reduce(reduce, BuiltinReds, KVs, [Count|Acc]);
builtin_reduce(rereduce, [<<"_count",_/binary>>|BuiltinReds], KVs, Acc) ->
- Count = builtin_sum_rows(KVs),
+ Count = builtin_sum_rows(KVs, []),
builtin_reduce(rereduce, BuiltinReds, KVs, [Count|Acc]);
builtin_reduce(Re, [<<"_stats",_/binary>>|BuiltinReds], KVs, Acc) ->
Stats = builtin_stats(Re, KVs),
builtin_reduce(Re, BuiltinReds, KVs, [Stats|Acc]).
-builtin_sum_rows(KVs) ->
- lists:foldl(fun([_Key, Value], Acc) -> sum_values(Value, Acc) end, 0, KVs).
-sum_values({Props}, 0) ->
- {Props};
-sum_values({Props}, {AccProps}) ->
- {sum_objects(lists:sort(Props), lists:sort(AccProps))};
+builtin_sum_rows([], Acc) ->
+ Acc;
+builtin_sum_rows([[_Key, Value] | RestKVs], Acc) ->
+ try sum_values(Value, Acc) of
+ NewAcc ->
+ builtin_sum_rows(RestKVs, NewAcc)
+ catch
+ throw:{builtin_reduce_error, Obj} ->
+ Obj;
+ throw:{invalid_value, Reason, Cause} ->
+ {[{<<"error">>, <<"builtin_reduce_error">>},
+ {<<"reason">>, Reason}, {<<"caused_by">>, Cause}]}
+ end.
+
+
sum_values(Value, Acc) when is_number(Value), is_number(Acc) ->
Acc + Value;
sum_values(Value, Acc) when is_list(Value), is_list(Acc) ->
@@ -169,6 +178,19 @@ sum_values(Value, Acc) when is_number(Value), is_list(Acc) ->
sum_arrays(Acc, [Value]);
sum_values(Value, Acc) when is_list(Value), is_number(Acc) ->
sum_arrays([Acc], Value);
+sum_values({Props}, Acc) ->
+ case lists:keyfind(<<"error">>, 1, Props) of
+ {<<"error">>, <<"builtin_reduce_error">>} ->
+ throw({builtin_reduce_error, {Props}});
+ false ->
+ ok
+ end,
+ case Acc of
+ 0 ->
+ {Props};
+ {AccProps} ->
+ {sum_objects(lists:sort(Props), lists:sort(AccProps))}
+ end;
sum_values(Else, _Acc) ->
throw_sum_error(Else).
@@ -490,7 +512,7 @@ ret_os_process(Proc) ->
ok.
throw_sum_error(Else) ->
- throw({invalid_value, iolist_to_binary(io_lib:format(?SUMERROR, [Else]))}).
+ throw({invalid_value, ?SUMERROR, Else}).
throw_stat_error(Else) ->
throw({invalid_value, iolist_to_binary(io_lib:format(?STATERROR, [Else]))}).
@@ -499,6 +521,17 @@ throw_stat_error(Else) ->
-ifdef(TEST).
-include_lib("eunit/include/eunit.hrl").
+builtin_sum_rows_negative_test() ->
+ A = [{[{<<"a">>, 1}]}, {[{<<"a">>, 2}]}, {[{<<"a">>, 3}]}],
+ E = {[{<<"error">>, <<"builtin_reduce_error">>}]},
+ ?assertEqual(E, builtin_sum_rows([["K", E]], [])),
+ % The below case is where the value is invalid, but no error because
+ % it's only one document.
+ ?assertEqual(A, builtin_sum_rows([["K", A]], [])),
+ {Result} = builtin_sum_rows([["K", A]], [1, 2, 3]),
+ ?assertEqual({<<"error">>, <<"builtin_reduce_error">>},
+ lists:keyfind(<<"error">>, 1, Result)).
+
sum_values_test() ->
?assertEqual(3, sum_values(1, 2)),
?assertEqual([2,4,6], sum_values(1, [1,4,6])),
@@ -512,6 +545,19 @@ sum_values_test() ->
?assertEqual(Z, sum_values(X, Y)),
?assertEqual(Z, sum_values(Y, X)).
+sum_values_negative_test() ->
+ % invalid value
+ A = [{[{<<"a">>, 1}]}, {[{<<"a">>, 2}]}, {[{<<"a">>, 3}]}],
+ B = ["error 1", "error 2"],
+ C = [<<"error 3">>, <<"error 4">>],
+ KV = {[{<<"error">>, <<"builtin_reduce_error">>},
+ {<<"reason">>, ?SUMERROR}, {<<"caused_by">>, <<"some cause">>}]},
+ ?assertThrow({invalid_value, _, _}, sum_values(A, [1, 2, 3])),
+ ?assertThrow({invalid_value, _, _}, sum_values(A, 0)),
+ ?assertThrow({invalid_value, _, _}, sum_values(B, [1, 2])),
+ ?assertThrow({invalid_value, _, _}, sum_values(C, [0])),
+ ?assertThrow({builtin_reduce_error, KV}, sum_values(KV, [0])).
+
stat_values_test() ->
?assertEqual({1, 2, 0, 1, 1}, stat_values(1, 0)),
?assertEqual({11, 2, 1, 10, 101}, stat_values(1, 10)),