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/20 20:00:12 UTC
[GitHub] [couchdb] nickva opened a new pull request #3093: Do not use (catch ...) in couch_views_reader:load_docs/4
nickva opened a new pull request #3093:
URL: https://github.com/apache/couchdb/pull/3093
Any error there would just be generating a case clause.
Remove the `_Else` catch-all clause too, as `open_doc_revs/4` should return a list of just one item that is either `{ok, Doc}` or `{{not_found, missing}, Rev}`. That's because it is called with just a single `[Rev1]` parameter and views `#mrargs` can't have the `latest` doc_options option.
References:
* [fabric2_db:open_doc_revs/4](https://github.com/apache/couchdb/blob/prototype/fdb-layer/src/fabric/src/fabric2_db.erl#L684-L691)
* [couch_mrview_http](https://github.com/apache/couchdb/blob/prototype/fdb-layer/src/couch_mrview/src/couch_mrview_http.erl#L491)
----------------------------------------------------------------
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] nickva commented on a change in pull request #3093: Do not use (catch ...) in couch_views_reader:load_docs/4
Posted by GitBox <gi...@apache.org>.
nickva commented on a change in pull request #3093:
URL: https://github.com/apache/couchdb/pull/3093#discussion_r474250298
##########
File path: src/couch_views/src/couch_views_reader.erl
##########
@@ -210,8 +210,7 @@ load_doc(TxDb, Id, null, DocOpts) ->
load_doc(TxDb, Id, Rev, DocOpts) ->
Rev1 = couch_doc:parse_rev(Rev),
- case (catch fabric2_db:open_doc_revs(TxDb, Id, [Rev1], DocOpts)) of
+ case fabric2_db:open_doc_revs(TxDb, Id, [Rev1], DocOpts) of
{ok, [{ok, Doc}]} -> couch_doc:to_json_obj(Doc, DocOpts);
- {ok, [{{not_found, missing}, Rev}]} -> null;
- {ok, [_Else]} -> null
+ {ok, [{{not_found, missing}, Rev}]} -> null
Review comment:
Good call
----------------------------------------------------------------
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] nickva merged pull request #3093: Do not use (catch ...) in couch_views_reader:load_docs/4
Posted by GitBox <gi...@apache.org>.
nickva merged pull request #3093:
URL: https://github.com/apache/couchdb/pull/3093
----------------------------------------------------------------
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 #3093: Do not use (catch ...) in couch_views_reader:load_docs/4
Posted by GitBox <gi...@apache.org>.
davisp commented on a change in pull request #3093:
URL: https://github.com/apache/couchdb/pull/3093#discussion_r474244227
##########
File path: src/couch_views/src/couch_views_reader.erl
##########
@@ -210,8 +210,7 @@ load_doc(TxDb, Id, null, DocOpts) ->
load_doc(TxDb, Id, Rev, DocOpts) ->
Rev1 = couch_doc:parse_rev(Rev),
- case (catch fabric2_db:open_doc_revs(TxDb, Id, [Rev1], DocOpts)) of
+ case fabric2_db:open_doc_revs(TxDb, Id, [Rev1], DocOpts) of
{ok, [{ok, Doc}]} -> couch_doc:to_json_obj(Doc, DocOpts);
- {ok, [{{not_found, missing}, Rev}]} -> null;
- {ok, [_Else]} -> null
+ {ok, [{{not_found, missing}, Rev}]} -> null
Review comment:
I don't think you want `Rev` here as it appears to be an accidental match against the string version of `Rev` as a function argument.
Looking through the code it appears we return either `{{not_found, missing}, {Pos, RevId}}` or `{ok, Doc}` from all scenarios with the caveat that I didn't chase down every `AfterDocRead` function to ensure they always return one of those forms as that would bubble up if it matched something weird.
I think what I'd do is probably just match` {ok, [{ok, Doc}]} -> ` and then a `{ok, _}` clause to return `null` as anything that's not `{ok, Doc}` will need to have `null`. Or we could fail the request in any situation that doesn't have the expected `{not_found, missing}` value.
----------------------------------------------------------------
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