You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by "pgj (via GitHub)" <gi...@apache.org> on 2023/05/02 13:40:56 UTC

[GitHub] [couchdb] pgj commented on a diff in pull request #4554: feat(`mango`): add `keys_examined` for `execution_stats` (part 1)

pgj commented on code in PR #4554:
URL: https://github.com/apache/couchdb/pull/4554#discussion_r1182565313


##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -457,12 +480,21 @@ handle_message({row, Props}, Cursor) ->
             couch_log:error("~s :: Error loading doc: ~p", [?MODULE, Error]),
             {ok, Cursor}
     end;
-handle_message({execution_stats, ShardStats}, #cursor{execution_stats = Stats} = Cursor) ->
-    {docs_examined, DocsExamined} = ShardStats,
-    Cursor1 = Cursor#cursor{
+% TODO: remove clause couchdb 4 -- mixed-version upgrade support
+handle_message(
+    {execution_stats, {docs_examined, DocsExamined}}, #cursor{execution_stats = Stats} = Cursor0

Review Comment:
   Thanks Nick for the comments!  I have had the reasons for these choices, let me add them for clarification:
   
   - The `{execution_stats, {docs_examined, DocsExamined}}` pattern was embedded into the function head to avoid the potential overlap with other clause.  My understanding is that heads are checked top-down and the body of the first matching one is going to be evaluated.  If there is no direct pattern to match on the contents of the argument of `execution_stats` (i.e. the second element in the tuple) in the head, the clause will be skipped and missed.
   
   -  For the second clause on `execution_stats`, I avoided to match on the type of `ShardStats` to keep it opaque.  Ideally, `mango_cursor_view:shard_stats_get/2` will handle the map check eventually.  I am aware that might be too late, but there are no other alternatives present to be confused with.  At the same time, changes to the format of shard stats is encapsulated into the implementation of the `shard_stats/2` and `shard_stats_get/2` functions.



-- 
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: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org