rnewson commented on code in PR #4414:
URL: https://github.com/apache/couchdb/pull/4414#discussion_r1140721653
##########
src/couch_mrview/src/couch_mrview_http.erl:
##########
@@ -43,6 +43,11 @@
-include_lib("couch/include/couch_db.hrl").
-include_lib("couch_mrview/include/couch_mrview.hrl").
+-define(DEFAULT_ENABLE_KEY_EXCLUSIVITY, false).
+-define(ERROR_KEY_INCOMPATIBLE,
+ {query_parse_error, <<"`key(s)` is incompatible with `start_key` and
`end_key`">>}
Review Comment:
can we be clearer and say "`key` and `keys` is incompatible..." please.
##########
src/couch_mrview/src/couch_mrview_http.erl:
##########
@@ -43,6 +43,11 @@
-include_lib("couch/include/couch_db.hrl").
-include_lib("couch_mrview/include/couch_mrview.hrl").
+-define(DEFAULT_ENABLE_KEY_EXCLUSIVITY, false).
Review Comment:
we do the defaults inline everywhere else and this define is used exactly
once here, so I would prefer to just say `false` in the config:get_boolean line
below.
##########
src/couch_mrview/src/couch_mrview_http.erl:
##########
@@ -486,8 +491,43 @@ parse_params(Props, Keys, #mrargs{} = Args0, Options) ->
Args0;
_ ->
% group_level set to undefined to detect if explicitly set by
user
- Args0#mrargs{keys = Keys, group = undefined, group_level =
undefined}
+ case Keys of
+ [_] ->
+ Args0#mrargs{group = undefined, group_level =
undefined};
+ _ ->
+ Args0#mrargs{keys = Keys, group = undefined,
group_level = undefined}
+ end
end,
+
+ PropsKeys = proplists:get_keys(Props),
+ case config:get_boolean("chttpd", "enable_key_exclusivity",
?DEFAULT_ENABLE_KEY_EXCLUSIVITY) of
Review Comment:
A separate function would be clearer as nick suggests. we can have separate
clauses for the `true` and `false` logic.
##########
src/couch_mrview/src/couch_mrview_http.erl:
##########
@@ -486,8 +491,43 @@ parse_params(Props, Keys, #mrargs{} = Args0, Options) ->
Args0;
_ ->
% group_level set to undefined to detect if explicitly set by
user
- Args0#mrargs{keys = Keys, group = undefined, group_level =
undefined}
+ case Keys of
+ [_] ->
+ Args0#mrargs{group = undefined, group_level =
undefined};
+ _ ->
+ Args0#mrargs{keys = Keys, group = undefined,
group_level = undefined}
+ end
end,
+
+ PropsKeys = proplists:get_keys(Props),
+ case config:get_boolean("chttpd", "enable_key_exclusivity",
?DEFAULT_ENABLE_KEY_EXCLUSIVITY) of
Review Comment:
I don't find the sets suggestion much tidier than the lists:foldl, how about;
```
lists:any(fun(K) -> lists:member(K, ["start_key", "startkey", "end_key",
"endkey"]) end, PropsKeys).
```
?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
