This is an automated email from the ASF dual-hosted git repository. vatamane pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 28480f9a438fa27e35219b9e09fcaaebe8010047 Author: Gabor Pali <[email protected]> AuthorDate: Mon Apr 17 19:49:34 2023 +0200 mango: fix definition of index coverage Covering indexes shall provide all the fields that the selector may contain, otherwise the derived documents would get dropped on the "match and extract" phase even if they were matching. Extend the integration tests to check this case as well. --- src/mango/src/mango_cursor_view.erl | 45 +++++++++++++++++++++++++++-- src/mango/src/mango_selector.erl | 49 +++++++++++++++++++++++++++++++- src/mango/test/22-covering-index-test.py | 18 ++++++++++-- 3 files changed, 107 insertions(+), 5 deletions(-) diff --git a/src/mango/src/mango_cursor_view.erl b/src/mango/src/mango_cursor_view.erl index 474d3bfe6..4dbea77c8 100644 --- a/src/mango/src/mango_cursor_view.erl +++ b/src/mango/src/mango_cursor_view.erl @@ -108,11 +108,18 @@ create(Db, Indexes, Selector, Opts) -> bookmark = Bookmark }}. +-spec required_fields(#cursor{}) -> fields(). +required_fields(#cursor{fields = all_fields}) -> + all_fields; +required_fields(#cursor{fields = Fields, selector = Selector}) -> + lists:usort(Fields ++ mango_selector:fields(Selector)). + -spec explain(#cursor{}) -> nonempty_list(term()). explain(#cursor{opts = Opts} = Cursor) -> BaseArgs = base_args(Cursor), Args0 = apply_opts(Opts, BaseArgs), - #cursor{index = Index, fields = Fields} = Cursor, + #cursor{index = Index} = Cursor, + Fields = required_fields(Cursor), Args = consider_index_coverage(Index, Fields, Args0), [ @@ -197,7 +204,8 @@ execute(#cursor{db = Db, index = Idx, execution_stats = Stats} = Cursor0, UserFu BaseArgs = base_args(Cursor), #cursor{opts = Opts, bookmark = Bookmark} = Cursor, Args0 = apply_opts(Opts, BaseArgs), - Args1 = consider_index_coverage(Idx, Cursor#cursor.fields, Args0), + Fields = required_fields(Cursor), + Args1 = consider_index_coverage(Idx, Fields, Args0), Args = mango_json_bookmark:update_args(Bookmark, Args1), UserCtx = couch_util:get_value(user_ctx, Opts, #user_ctx{}), DbOpts = [{user_ctx, UserCtx}], @@ -859,6 +867,39 @@ create_test() -> }, ?assertEqual({ok, Cursor}, create(db, Indexes, Selector, Options)). +to_selector(Map) -> + test_util:as_selector(Map). + +required_fields_all_fields_test() -> + Cursor = #cursor{fields = all_fields}, + ?assertEqual(all_fields, required_fields(Cursor)). + +required_fields_disjoint_fields_test() -> + Fields1 = [<<"field1">>, <<"field2">>, <<"field3">>], + Selector1 = to_selector(#{}), + Cursor1 = #cursor{fields = Fields1, selector = Selector1}, + ?assertEqual([<<"field1">>, <<"field2">>, <<"field3">>], required_fields(Cursor1)), + Fields2 = [<<"field1">>, <<"field2">>], + Selector2 = to_selector(#{<<"field3">> => undefined, <<"field4">> => undefined}), + Cursor2 = #cursor{fields = Fields2, selector = to_selector(Selector2)}, + ?assertEqual( + [<<"field1">>, <<"field2">>, <<"field3">>, <<"field4">>], required_fields(Cursor2) + ). + +required_fields_overlapping_fields_test() -> + 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) + ), + 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) + ). + explain_test() -> Cursor = #cursor{ diff --git a/src/mango/src/mango_selector.erl b/src/mango/src/mango_selector.erl index 7de16bd51..59be7a6eb 100644 --- a/src/mango/src/mango_selector.erl +++ b/src/mango/src/mango_selector.erl @@ -16,7 +16,8 @@ normalize/1, match/2, has_required_fields/2, - is_constant_field/2 + is_constant_field/2, + fields/1 ]). -include_lib("couch/include/couch_db.hrl"). @@ -638,6 +639,14 @@ is_constant_field([{[{Field, {[{Cond, _Val}]}}]} | _Rest], Field) -> is_constant_field([{[{_UnMatched, _}]} | Rest], Field) -> is_constant_field(Rest, Field). +-spec fields(selector()) -> fields(). +fields({[{<<"$", _/binary>>, Args}]}) when is_list(Args) -> + lists:flatmap(fun fields/1, Args); +fields({[{Field, _Cond}]}) -> + [Field]; +fields({[]}) -> + []. + %%%%%%%% module tests below %%%%%%%% -ifdef(TEST). @@ -1007,4 +1016,42 @@ match_demo_test_() -> ?_assertEqual(false, Check({[{<<"_id">>, <<"foo">>}, {<<"_rev">>, <<"quux">>}]})) ]. +fields_of(Selector) -> + fields(test_util:as_selector(Selector)). + +fields_empty_test() -> + ?assertEqual([], fields_of(#{})). + +fields_primitive_test() -> + Selector = #{<<"field">> => undefined}, + ?assertEqual([<<"field">>], fields_of(Selector)). + +fields_nested_test() -> + Selector = #{<<"field1">> => #{<<"field2">> => undefined}}, + ?assertEqual([<<"field1.field2">>], fields_of(Selector)). + +fields_and_test() -> + Selector1 = #{<<"$and">> => []}, + ?assertEqual([], fields_of(Selector1)), + Selector2 = #{ + <<"$and">> => [#{<<"field1">> => undefined}, #{<<"field2">> => undefined}] + }, + ?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)). + +fields_or_test() -> + Selector1 = #{<<"$or">> => []}, + ?assertEqual([], fields_of(Selector1)), + Selector2 = #{ + <<"$or">> => [#{<<"field1">> => undefined}, #{<<"field2">> => undefined}] + }, + ?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)). + +fields_nor_test() -> + Selector1 = #{<<"$nor">> => []}, + ?assertEqual([], fields_of(Selector1)), + Selector2 = #{ + <<"$nor">> => [#{<<"field1">> => undefined}, #{<<"field2">> => undefined}] + }, + ?assertEqual([<<"field1">>, <<"field2">>], fields_of(Selector2)). + -endif. diff --git a/src/mango/test/22-covering-index-test.py b/src/mango/test/22-covering-index-test.py index b2f0202ed..52a7f3612 100644 --- a/src/mango/test/22-covering-index-test.py +++ b/src/mango/test/22-covering-index-test.py @@ -21,8 +21,8 @@ class CoveringIndexTests(mango.UserDocsTests): self.assertEqual(resp["mrargs"]["include_docs"], False) self.assertEqual(resp["covered"], True) - def is_not_covered(self, selector, fields): - resp = self.db.find(selector, fields=fields, explain=True) + def is_not_covered(self, selector, fields, use_index=None): + resp = self.db.find(selector, fields=fields, use_index=use_index, explain=True) self.assertEqual(resp["mrargs"]["include_docs"], True) self.assertEqual(resp["covered"], False) @@ -74,6 +74,20 @@ class CoveringIndexTests(mango.UserDocsTests): def test_index_does_not_cover_query_partial_selector(self): self.is_not_covered({"name.last": "Hernandez"}, ["name.first"]) + def test_index_does_not_cover_selector_with_more_fields(self): + self.is_not_covered( + { + "$and": [ + {"age": {"$ne": 23}}, + {"twitter": {"$not": {"$regex": "^@.*[0-9]+$"}}}, + {"location.address.number": {"$gt": 4288}}, + {"location.city": {"$ne": "Pico Rivera"}}, + ] + }, + ["twitter"], + use_index="twitter", + ) + def test_covering_index_provides_correct_answer_id(self): docs = self.db.find({"age": {"$gte": 32}}, fields=["_id"]) expected = [
