fix: Hide the format of selector fields inside mango_selector

In the previous commit we changed lots of code to deal with normalized
selector fields now being per-parsed lists, not binaries, i.e. instead
of `<<"a.b">>` a field now looks like `[<<"a">>, <<"b">>]`.

This commit winds most of that back and instead makes the functions in
`mango_selector` hide the fact the field is now pre-parsed from callers.
So for example, the code that deals with matching selectors to indexes
can continue to pass the unparsed form into `mango_selector` functions,
and `mango_selector` will take care of parsing/serializing inputs as
necessary.

Author: jcoglan

src/mango/src/mango_cursor.erl

@@ -286,10 +286,8 @@ extract_selector_hints(Selector) ->
             UnindexableFields = sets:subtract(AllFields, IndexableFields),
             {[
                 {type, IndexType},
-                {indexable_fields,
-                    lists:sort([mango_util:join_field(F) || F <- sets:to_list(IndexableFields)])},
-                {unindexable_fields,
-                    lists:sort([mango_util:join_field(F) || F <- sets:to_list(UnindexableFields)])}
+                {indexable_fields, lists:sort(sets:to_list(IndexableFields))},
+                {unindexable_fields, lists:sort(sets:to_list(UnindexableFields))}
             ]}
         end,
     lists:map(Populate, Modules).
@@ -1114,56 +1112,56 @@ extract_selector_hints_test_() ->
 t_extract_selector_hints_view(_) ->
     meck:expect(dreyfus, available, [], meck:val(false)),
     meck:expect(nouveau, enabled, [], meck:val(false)),
-    meck:expect(mango_selector, fields, [selector], meck:val([["field1"], ["field2"], ["field3"]])),
-    meck:expect(mango_idx_view, indexable_fields, [selector], meck:val([["field2"]])),
+    meck:expect(mango_selector, fields, [selector], meck:val(["field1", "field2", "field3"])),
+    meck:expect(mango_idx_view, indexable_fields, [selector], meck:val(["field2"])),
     Hints =
         [
             {[
                 {type, json},
-                {indexable_fields, [<<"field2">>]},
-                {unindexable_fields, [<<"field1">>, <<"field3">>]}
+                {indexable_fields, ["field2"]},
+                {unindexable_fields, ["field1", "field3"]}
             ]}
         ],
     ?assertEqual(Hints, extract_selector_hints(selector)).
 
 t_extract_selector_hints_text(_) ->
     meck:expect(dreyfus, available, [], meck:val(true)),
     meck:expect(nouveau, enabled, [], meck:val(false)),
-    meck:expect(mango_selector, fields, [selector], meck:val([["field1"], ["field2"], ["field3"]])),
-    meck:expect(mango_idx_view, indexable_fields, [selector], meck:val([["field2"]])),
-    meck:expect(mango_idx_text, indexable_fields, [selector], meck:val([["field1"]])),
+    meck:expect(mango_selector, fields, [selector], meck:val(["field1", "field2", "field3"])),
+    meck:expect(mango_idx_view, indexable_fields, [selector], meck:val(["field2"])),
+    meck:expect(mango_idx_text, indexable_fields, [selector], meck:val(["field1"])),
     Hints =
         [
             {[
                 {type, json},
-                {indexable_fields, [<<"field2">>]},
-                {unindexable_fields, [<<"field1">>, <<"field3">>]}
+                {indexable_fields, ["field2"]},
+                {unindexable_fields, ["field1", "field3"]}
             ]},
             {[
                 {type, text},
-                {indexable_fields, [<<"field1">>]},
-                {unindexable_fields, [<<"field2">>, <<"field3">>]}
+                {indexable_fields, ["field1"]},
+                {unindexable_fields, ["field2", "field3"]}
             ]}
         ],
     ?assertEqual(Hints, extract_selector_hints(selector)).
 
 t_extract_selector_hints_nouveau(_) ->
     meck:expect(dreyfus, available, [], meck:val(false)),
     meck:expect(nouveau, enabled, [], meck:val(true)),
-    meck:expect(mango_selector, fields, [selector], meck:val([["field1"], ["field2"], ["field3"]])),
-    meck:expect(mango_idx_view, indexable_fields, [selector], meck:val([["field2"]])),
-    meck:expect(mango_idx_nouveau, indexable_fields, [selector], meck:val([["field1"]])),
+    meck:expect(mango_selector, fields, [selector], meck:val(["field1", "field2", "field3"])),
+    meck:expect(mango_idx_view, indexable_fields, [selector], meck:val(["field2"])),
+    meck:expect(mango_idx_nouveau, indexable_fields, [selector], meck:val(["field1"])),
     Hints =
         [
             {[
                 {type, json},
-                {indexable_fields, [<<"field2">>]},
-                {unindexable_fields, [<<"field1">>, <<"field3">>]}
+                {indexable_fields, ["field2"]},
+                {unindexable_fields, ["field1", "field3"]}
             ]},
             {[
                 {type, nouveau},
-                {indexable_fields, [<<"field1">>]},
-                {unindexable_fields, [<<"field2">>, <<"field3">>]}
+                {indexable_fields, ["field1"]},
+                {unindexable_fields, ["field2", "field3"]}
             ]}
         ],
     ?assertEqual(Hints, extract_selector_hints(selector)).

src/mango/src/mango_cursor_special.erl

@@ -33,7 +33,7 @@
     Options :: cursor_options().
 create(Db, {Indexes, Trace0}, Selector, Opts) ->
     InitialRange = mango_idx_view:field_ranges(Selector),
-    CatchAll = [{[<<"_id">>], {'$gt', null, '$lt', mango_json_max}}],
+    CatchAll = [{<<"_id">>, {'$gt', null, '$lt', mango_json_max}}],
     % order matters here - we only want to use the catchall index
     % if no other range can fulfill the query (because we know)
     % catchall is the most expensive range

src/mango/src/mango_cursor_view.erl

@@ -882,7 +882,7 @@ consider_index_coverage_positive_test() ->
             type = <<"json">>,
             def = {[{<<"fields">>, {[]}}]}
         },
-    Fields = [[<<"_id">>]],
+    Fields = [<<"_id">>],
     MRArgs =
         #mrargs{
             include_docs = true,
@@ -1010,29 +1010,29 @@ required_fields_all_fields_test() ->
     ?assertEqual(all_fields, required_fields(Cursor)).
 
 required_fields_disjoint_fields_test() ->
-    Fields1 = [[<<"field1">>], [<<"field2">>], [<<"field3">>]],
+    Fields1 = [<<"field1">>, <<"field2">>, <<"field3">>],
     Selector1 = to_selector(#{}),
     Cursor1 = #cursor{fields = Fields1, selector = Selector1},
-    ?assertEqual([[<<"field1">>], [<<"field2">>], [<<"field3">>]], required_fields(Cursor1)),
-    Fields2 = [[<<"field1">>], [<<"field2">>]],
+    ?assertEqual([<<"field1">>, <<"field2">>, <<"field3">>], required_fields(Cursor1)),
+    Fields2 = [<<"field1">>, <<"field2">>],
     Selector2 = to_selector(#{<<"field3">> => undefined, <<"field4">> => undefined}),
     Cursor2 = #cursor{fields = Fields2, selector = Selector2},
     ?assertEqual(
-        [[<<"field1">>], [<<"field2">>], [<<"field3">>], [<<"field4">>]], required_fields(Cursor2)
+        [<<"field1">>, <<"field2">>, <<"field3">>, <<"field4">>], required_fields(Cursor2)
     ).
 
 required_fields_overlapping_fields_test() ->
-    Fields1 = [[<<"field1">>], [<<"field2">>], [<<"field3">>]],
+    Fields1 = [<<"field1">>, <<"field2">>, <<"field3">>],
     Selector1 = to_selector(#{<<"field3">> => undefined, <<"field4">> => undefined}),
     Cursor1 = #cursor{fields = Fields1, selector = Selector1},
     ?assertEqual(
-        [[<<"field1">>], [<<"field2">>], [<<"field3">>], [<<"field4">>]], required_fields(Cursor1)
+        [<<"field1">>, <<"field2">>, <<"field3">>, <<"field4">>], required_fields(Cursor1)
     ),
-    Fields2 = [[<<"field3">>], [<<"field1">>], [<<"field2">>]],
+    Fields2 = [<<"field3">>, <<"field1">>, <<"field2">>],
     Selector2 = to_selector(#{<<"field4">> => undefined, <<"field1">> => undefined}),
     Cursor2 = #cursor{fields = Fields2, selector = Selector2},
     ?assertEqual(
-        [[<<"field1">>], [<<"field2">>], [<<"field3">>], [<<"field4">>]], required_fields(Cursor2)
+        [<<"field1">>, <<"field2">>, <<"field3">>, <<"field4">>], required_fields(Cursor2)
     ).
 
 explain_test() ->

src/mango/src/mango_idx_special.erl

@@ -55,20 +55,13 @@ to_json(#idx{def = all_docs}) ->
     ]}.
 
 columns(#idx{def = all_docs}) ->
-    [[<<"_id">>]].
-
-parse_field(Field) when is_binary(Field) ->
-    {ok, Path} = mango_util:parse_field(Field),
-    Path;
-parse_field(Field) ->
-    Field.
+    [<<"_id">>].
 
 is_usable(#idx{def = all_docs}, _Selector, []) ->
     {true, #{reason => []}};
-is_usable(#idx{def = all_docs} = Idx, Selector, SortFields0) ->
+is_usable(#idx{def = all_docs} = Idx, Selector, SortFields) ->
     Fields = mango_idx_view:indexable_fields(Selector),
-    SortFields = [parse_field(F) || F <- SortFields0],
-    SelectorHasRequiredFields = lists:member([<<"_id">>], Fields),
+    SelectorHasRequiredFields = lists:member(<<"_id">>, Fields),
     CanUseSort = can_use_sort(Idx, SortFields, Selector),
     Reason =
         [field_mismatch || not SelectorHasRequiredFields] ++
@@ -124,7 +117,7 @@ is_usable_test() ->
     SortOrderMismatch = {false, #{reason => [sort_order_mismatch]}},
 
     Index = #idx{def = all_docs},
-    ?assertEqual(FieldMismatch, usable(Index, #{}, [[<<"_id">>]])),
+    ?assertEqual(FieldMismatch, usable(Index, #{}, [<<"_id">>])),
     ?assertEqual(SortOrderMismatch, usable(Index, #{<<"_id">> => 11}, [<<"field3">>])),
     ?assertEqual(Usable, usable(Index, #{}, [])).
 -endif.

src/mango/src/mango_idx_view.erl

@@ -111,31 +111,24 @@ to_json(Idx) ->
 columns(Idx) ->
     {Props} = Idx#idx.def,
     {<<"fields">>, {Fields}} = lists:keyfind(<<"fields">>, 1, Props),
-    [parse_field(F) || {F, _} <- Fields].
-
-parse_field(Field) when is_binary(Field) ->
-    {ok, Path} = mango_util:parse_field(Field),
-    Path;
-parse_field(Field) ->
-    Field.
+    [Key || {Key, _} <- Fields].
 
 -spec is_usable(#idx{}, selector(), [field()]) -> {boolean(), rejection_details()}.
-is_usable(Idx, Selector, SortFields0) ->
+is_usable(Idx, Selector, SortFields) ->
     % This index is usable if all of the columns are
     % restricted by the selector such that they are required to exist
     % and the selector is not a text search (so requires a text index)
     RequiredFields = columns(Idx),
 
     % sort fields are required to exist in the results so
     % we don't need to check the selector for these
-    SortFields = [parse_field(F) || F <- SortFields0],
     RequiredFields1 = ordsets:subtract(lists:usort(RequiredFields), lists:usort(SortFields)),
 
     % _id and _rev are implicitly in every document so
     % we don't need to check the selector for these either
     RequiredFields2 = ordsets:subtract(
         RequiredFields1,
-        [[<<"_id">>], [<<"_rev">>]]
+        [<<"_id">>, <<"_rev">>]
     ),
 
     SelectorHasRequiredFields = mango_selector:has_required_fields(Selector, RequiredFields2),
@@ -278,23 +271,26 @@ validate_ddoc(VProps) ->
 % the equivalent of a multi-query. But that's for another
 % day.
 
+indexable_fields(Selector) ->
+    [mango_util:join_field(F) || F <- indexable_paths(Selector)].
+
 % We can see through '$and' trivially
-indexable_fields({[{<<"$and">>, Args}]}) ->
-    lists:usort(lists:flatmap(fun(A) -> indexable_fields(A) end, Args));
+indexable_paths({[{<<"$and">>, Args}]}) ->
+    lists:usort(lists:flatmap(fun(A) -> indexable_paths(A) end, Args));
 % So far we can't see through any other operator
-indexable_fields({[{<<"$", _/binary>>, _}]}) ->
+indexable_paths({[{<<"$", _/binary>>, _}]}) ->
     [];
 % If we have a field with a terminator that is locatable
 % using an index then the field is a possible index
-indexable_fields({[{Field, Cond}]}) ->
+indexable_paths({[{Field, Cond}]}) ->
     case indexable(Cond) of
         true ->
             [Field];
         false ->
             []
     end;
 % An empty selector
-indexable_fields({[]}) ->
+indexable_paths({[]}) ->
     [].
 
 % Check if a condition is indexable. The logical
@@ -327,8 +323,8 @@ indexable({[{<<"$", _/binary>>, _}]}) ->
 
 % For each field, return {Field, Range}
 field_ranges(Selector) ->
-    Fields = indexable_fields(Selector),
-    field_ranges(Selector, Fields).
+    Fields = indexable_paths(Selector),
+    [{mango_util:join_field(F), R} || {F, R} <- field_ranges(Selector, Fields)].
 
 field_ranges(Selector, Fields) ->
     field_ranges(Selector, Fields, []).
@@ -555,13 +551,12 @@ can_use_sort([Col | RestCols], SortFields, Selector) ->
 -spec covers(#idx{}, fields()) -> boolean().
 covers(_, all_fields) ->
     false;
-covers(Idx, Fields0) ->
+covers(Idx, Fields) ->
     case mango_idx:def(Idx) of
         all_docs ->
             false;
         _ ->
-            Fields = [parse_field(F) || F <- Fields0],
-            Available = [[<<"_id">>] | columns(Idx)],
+            Available = [<<"_id">> | columns(Idx)],
             sets:is_subset(couch_util:set_from_list(Fields), couch_util:set_from_list(Available))
     end.
 
@@ -576,7 +571,7 @@ indexable_fields_empty_test() ->
 
 indexable_fields_and_test() ->
     Selector = #{<<"$and">> => [#{<<"field1">> => undefined, <<"field2">> => undefined}]},
-    ?assertEqual([[<<"field1">>], [<<"field2">>]], indexable_fields_of(Selector)).
+    ?assertEqual([<<"field1">>, <<"field2">>], indexable_fields_of(Selector)).
 
 indexable_fields_or_test() ->
     Selector = #{<<"$or">> => [#{<<"field1">> => undefined, <<"field2">> => undefined}]},
@@ -616,31 +611,31 @@ indexable_fields_not_test() ->
 
 indexable_fields_lt_test() ->
     Selector = #{<<"field">> => #{<<"$lt">> => undefined}},
-    ?assertEqual([[<<"field">>]], indexable_fields_of(Selector)).
+    ?assertEqual([<<"field">>], indexable_fields_of(Selector)).
 
 indexable_fields_lte_test() ->
     Selector = #{<<"field">> => #{<<"$lte">> => undefined}},
-    ?assertEqual([[<<"field">>]], indexable_fields_of(Selector)).
+    ?assertEqual([<<"field">>], indexable_fields_of(Selector)).
 
 indexable_fields_eq_test() ->
     Selector = #{<<"field">> => #{<<"$eq">> => undefined}},
-    ?assertEqual([[<<"field">>]], indexable_fields_of(Selector)).
+    ?assertEqual([<<"field">>], indexable_fields_of(Selector)).
 
 indexable_fields_ne_test() ->
     Selector = #{<<"field">> => #{<<"$ne">> => undefined}},
     ?assertEqual([], indexable_fields_of(Selector)).
 
 indexable_fields_gte_test() ->
     Selector = #{<<"field">> => #{<<"$gte">> => undefined}},
-    ?assertEqual([[<<"field">>]], indexable_fields_of(Selector)).
+    ?assertEqual([<<"field">>], indexable_fields_of(Selector)).
 
 indexable_fields_beginswith_test() ->
     Selector = #{<<"field">> => #{<<"$beginsWith">> => undefined}},
-    ?assertEqual([[<<"field">>]], indexable_fields_of(Selector)).
+    ?assertEqual([<<"field">>], indexable_fields_of(Selector)).
 
 indexable_fields_gt_test() ->
     Selector = #{<<"field">> => #{<<"$gt">> => undefined}},
-    ?assertEqual([[<<"field">>]], indexable_fields_of(Selector)).
+    ?assertEqual([<<"field">>], indexable_fields_of(Selector)).
 
 indexable_fields_mod_test() ->
     Selector = #{<<"field">> => #{<<"$mod">> => [0, 0]}},
@@ -652,7 +647,7 @@ indexable_fields_regex_test() ->
 
 indexable_fields_exists_test() ->
     Selector = #{<<"field">> => #{<<"$exists">> => true}},
-    ?assertEqual([[<<"field">>]], indexable_fields_of(Selector)).
+    ?assertEqual([<<"field">>], indexable_fields_of(Selector)).
 
 indexable_fields_type_test() ->
     Selector = #{<<"field">> => #{<<"$type">> => undefined}},
@@ -715,7 +710,7 @@ is_usable_test() ->
     ?assertEqual(
         SortOrderMismatch,
         usable(Index, #{<<"field1">> => <<"value1">>, <<"field2">> => 42}, [
-            [<<"field3">>], [<<"field4">>]
+            <<"field3">>, <<"field4">>
         ])
     ),
     ?assertEqual(