nickva commented on code in PR #5869:
URL: https://github.com/apache/couchdb/pull/5869#discussion_r2931883216


##########
src/mango/src/mango_util.erl:
##########
@@ -370,6 +372,31 @@ parse_field_slow(Field) ->
     ),
     {ok, Path}.
 
+join_keys({Sel}) when is_list(Sel) ->
+    Pairs = [{join_field(K), join_keys(V)} || {K, V} <- Sel],
+    {Pairs};
+join_keys(Sel) when is_list(Sel) ->
+    [join_keys(S) || S <- Sel];
+join_keys(Sel) ->
+    Sel.
+
+join_field(Field) when is_list(Field) ->
+    Parts = [field_to_binary(F) || F <- Field],
+    binary_join(Parts, <<".">>);
+join_field(Field) ->
+    Field.
+
+% binary:join/2 is not available on all Erlang versions we support; it was
+% added in 28.0. For now, we use this function in its place, c.f.
+% https://www.erlang.org/doc/apps/stdlib/binary.html#join/2
+binary_join(Binaries, Separator) ->

Review Comment:
   Could add a guard that Binaries is a list and Separator is a binary. 
Sometimes those asserts could also help the compiler to optimize some snippets.



##########
src/mango/src/mango_util.erl:
##########
@@ -370,6 +372,31 @@ parse_field_slow(Field) ->
     ),
     {ok, Path}.
 
+join_keys({Sel}) when is_list(Sel) ->
+    Pairs = [{join_field(K), join_keys(V)} || {K, V} <- Sel],
+    {Pairs};
+join_keys(Sel) when is_list(Sel) ->
+    [join_keys(S) || S <- Sel];
+join_keys(Sel) ->
+    Sel.
+
+join_field(Field) when is_list(Field) ->
+    Parts = [field_to_binary(F) || F <- Field],
+    binary_join(Parts, <<".">>);
+join_field(Field) ->
+    Field.
+
+% binary:join/2 is not available on all Erlang versions we support; it was

Review Comment:
   Since binary:join/2 is probably a faster implementation I wonder if we still 
get a noticeably indexing or filtering boost from this PR?
   
   Try indexing a few million docs and see if notice a difference



-- 
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]

Reply via email to