Skip to content

feat(mango): keys-only covering indexes #4512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nickva merged 11 commits into from
Apr 19, 2023
Merged

feat(mango): keys-only covering indexes #4512

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1efeb75
Remove explicit import
pgj Apr 17, 2023
21b7927
mango: introduce support for covering indexes
pgj Mar 16, 2023
4ed65d5
mango: add type information for better self-documentation
pgj Mar 20, 2023
7278574
mango: increase coverage of the `choose_best_index/1` test
pgj Mar 22, 2023
0394b9e
mango: add eunit tests
pgj Mar 22, 2023
96d0480
_find: mention the `covered` attribute in the `_explain` response
pgj Mar 22, 2023
7cbcce4
mango: add integration tests for keys-only covering indexes
pgj Mar 24, 2023
cfd4c73
mango: mark fields with the `$exists` operator indexable
pgj Mar 27, 2023
138fdfe
mango: enhance compositionality of `consider_index_coverage/3`
pgj Mar 27, 2023
842663d
mango: fix definition of index coverage
pgj Apr 17, 2023
cb9a3bf
mango: refactor
pgj Apr 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/couch/src/couch_util.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
-export([to_binary/1, to_integer/1, to_list/1, url_encode/1]).
-export([json_encode/1, json_decode/1, json_decode/2]).
-export([verify/2, simple_call/2, shutdown_sync/1]).
-export([get_value/2, get_value/3]).
-export([get_value/2, get_value/3, set_value/3]).
-export([reorder_results/2, reorder_results/3]).
-export([url_strip_password/1]).
-export([encode_doc_id/1]).
Expand Down Expand Up @@ -263,6 +263,9 @@ get_value(Key, List, Default) ->
Default
end.

set_value(Key, List, Value) ->
lists:keyreplace(Key, 1, List, {Key, Value}).

get_nested_json_value({Props}, [Key | Keys]) ->
case couch_util:get_value(Key, Props, nil) of
nil -> throw({not_found, <<"missing json key: ", Key/binary>>});
Expand Down
4 changes: 4 additions & 0 deletions src/docs/src/api/database/find.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1317,6 +1317,9 @@ it easier to take advantage of future improvements to query planning
:>header Content-Type: - :mimetype:`application/json`
:>header Transfer-Encoding: ``chunked``

:>json boolean covered: Tell if the query could be answered only
by relying on the data stored in the index. When ``true``, no
documents are fetched, which results in a faster response.
:>json string dbname: Name of database.
:>json object index: Index used to fulfill the query.
:>json object selector: Query selector used.
Expand Down Expand Up @@ -1364,6 +1367,7 @@ it easier to take advantage of future improvements to query planning
Transfer-Encoding: chunked

{
"covered": false,
"dbname": "movies",
"index": {
"ddoc": "_design/0d61d9177426b1e2aa8d0fe732ec6e506f5d443c",
Expand Down
12 changes: 12 additions & 0 deletions src/mango/src/mango.hrl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

-define(MANGO_ERROR(R), throw({mango_error, ?MODULE, R})).

-type maybe(A) :: A | undefined.
Copy link
Contributor

@nickva nickva Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is a bit as A can be undefined also. I'd would expect Dialyzer should emit some kind of warning here but perhaps our Dialyzer options are not configured properly.

Copy link
Contributor Author

@pgj pgj Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not see warnings from Dialyzer with regard to this definition. Though I have run it only with the standard options.

Copy link
Contributor

@nickva nickva Apr 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you run Dialyzer? Every time I run it either shows no output or crashes in a warning in config or a different application.

Copy link
Contributor Author

@pgj pgj Apr 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initialized the ~/.dialyzer_plt file by running the standard command:

$ dialyzer --build_plt --apps erts kernel stdlib mnesia
  Creating PLT /Users/gaborpali/.dialyzer_plt ...
Unknown functions:
  compile:file/2 (c.erl:385:10)
  compile:forms/2 (escript.erl:204:10)
  compile:noenv_forms/2 (erl_abstract_code.erl:10:9)
  compile:noenv_forms/2 (qlc_pt.erl:444:14)
  compile:output_generated/1 (c.erl:444:10)
  crypto:crypto_one_time/5 (beam_lib.erl:987:11)
  crypto:start/0 (beam_lib.erl:1023:10)
  crypto:strong_rand_bytes/1 (net_kernel.erl:1918:37)
Unknown types:
  compile:option/0 (c.erl:92:19)
  compile:option/0 (erl_expand_records.erl:41:26)
  compile:option/0 (erl_lint.erl:50:47)
  compile:option/0 (qlc.erl:541:32)
  compile:option/0 (qlc_pt.erl:73:32)
 done in 0m21.58s
done (passed successfully)

Then I was able to run Dialyzer only on the Mango portion of the code base:

$ dialyzer src/mango/ebin
  Checking whether the PLT /Users/gaborpali/.dialyzer_plt is up-to-date... yes
  Proceeding with analysis...
mango_epi.erl:15:2: Callback info about the couch_epi_plugin behaviour is not available
Unknown functions:
  chttpd:chunked_response_buffer_size/0 (src/mango_httpd.erl:295:21)
  chttpd:close_delayed_json_object/4 (src/mango_httpd.erl:276:19)
  chttpd:end_delayed_json_response/1 (src/mango_httpd.erl:288:5)
  chttpd:error_info/1 (src/mango_crud.erl:143:33)
  chttpd:json_body_obj/1 (src/mango_httpd.erl:127:50)
  chttpd:qs/1 (src/mango_httpd.erl:59:9)
  chttpd:qs_value/3 (src/mango_httpd.erl:241:16)
  chttpd:send_delayed_chunk/2 (src/mango_httpd.erl:287:19)
  chttpd:send_error/2 (src/mango_httpd.erl:41:21)
  chttpd:send_error/4 (src/mango_httpd.erl:43:21)
  chttpd:send_json/2 (src/mango_httpd.erl:185:13)
  chttpd:send_method_not_allowed/2 (src/mango_httpd.erl:192:5)
  chttpd:start_delayed_json_response/4 (src/mango_httpd.erl:272:5)
  chttpd:validate_ctype/2 (src/mango_httpd.erl:126:5)
  config:get/3 (src/mango_idx_text.erl:399:5)
  config:get_integer/3 (src/mango_cursor.erl:215:17)
  config:get_integer/3 (src/mango_cursor_text.erl:310:5)
  config:get_integer/3 (src/mango_opts.erl:360:5)
  couch_db:get_user_ctx/1 (src/mango_crud.erl:114:25)
  couch_db:get_user_ctx/1 (src/mango_idx_text.erl:384:30)
  couch_db:name/1 (src/mango_cursor_text.erl:52:28)
  couch_db:name/1 (src/mango_idx.erl:277:5)
  couch_db:name/1 (src/mango_idx_text.erl:383:14)
  couch_db:set_user_ctx/2 (src/mango_httpd.erl:220:19)
  couch_doc:from_json_obj/1 (src/mango_crud.erl:63:27)
  couch_doc:from_json_obj/1 (src/mango_doc.erl:46:11)
  couch_doc:rev_to_str/1 (src/mango_crud.erl:137:15)
  couch_doc:to_json_obj/2 (src/mango_cursor_view.erl:627:27)
  couch_ejson_compare:less/2 (src/mango_json.erl:47:5)
  couch_epi:register_service/2 (src/mango_sup.erl:23:33)
  couch_log:error/2 (src/mango_cursor_view.erl:457:13)
  couch_log:error/2 (src/mango_idx_text.erl:219:13)
  couch_log:error/2 (src/mango_idx_view.erl:245:13)
  couch_log:error/2 (src/mango_native_proc.erl:71:17)
  couch_log:error/2 (src/mango_util.erl:141:13)
  couch_log:warning/2 (src/mango_idx_text.erl:390:13)
  couch_mrview_util:get_extra/3 (src/mango_cursor_view.erl:136:29)
  couch_mrview_util:set_extra/3 (src/mango_cursor_view.erl:571:15)
  couch_partition:validate_partition/1 (src/mango_opts.erl:311:5)
  couch_stats:increment_counter/1 (src/mango_cursor.erl:164:5)
  couch_stats:increment_counter/1 (src/mango_cursor_text.erl:183:5)
  couch_stats:increment_counter/1 (src/mango_cursor_view.erl:381:13)
  couch_stats:increment_counter/1 (src/mango_execution_stats.erl:57:5)
  couch_stats:increment_counter/1 (src/mango_selector.erl:53:5)
  couch_stats:update_histogram/2 (src/mango_execution_stats.erl:76:5)
  couch_util:decodeBase64Url/1 (src/mango_json_bookmark.erl:55:35)
  couch_util:encodeBase64Url/1 (src/mango_json_bookmark.erl:47:5)
  couch_util:get_value/2 (src/mango_crud.erl:127:10)
  couch_util:get_value/2 (src/mango_cursor_special.erl:40:16)
  couch_util:get_value/2 (src/mango_cursor_text.erl:247:10)
  couch_util:get_value/2 (src/mango_cursor_view.erl:95:16)
  couch_util:get_value/2 (src/mango_execution_stats.erl:78:10)
  couch_util:get_value/2 (src/mango_httpd.erl:255:18)
  couch_util:get_value/2 (src/mango_idx.erl:353:17)
  couch_util:get_value/2 (src/mango_idx_text.erl:385:14)
  couch_util:get_value/2 (src/mango_native_proc.erl:115:16)
  couch_util:get_value/3 (src/mango_cursor_special.erl:39:14)
  couch_util:get_value/3 (src/mango_cursor_text.erl:57:14)
  couch_util:get_value/3 (src/mango_cursor_view.erl:94:14)
  couch_util:get_value/3 (src/mango_httpd.erl:72:14)
  couch_util:get_value/3 (src/mango_idx.erl:404:10)
  couch_util:get_value/3 (src/mango_idx_text.erl:113:23)
  couch_util:get_value/3 (src/mango_native_proc.erl:154:13)
  couch_util:json_decode/1 (src/mango_idx.erl:137:13)
  couch_util:json_encode/1 (src/mango_error.erl:55:45)
  couch_util:json_encode/1 (src/mango_httpd.erl:280:19)
  couch_util:json_encode/1 (src/mango_idx.erl:137:26)
  couch_util:set_value/3 (src/mango_cursor_view.erl:593:21)
  couch_uuids:new/0 (src/mango_crud.erl:67:46)
  couch_uuids:new/0 (src/mango_doc.erl:49:26)
  crypto:hash/2 (src/mango_idx.erl:327:11)
  ddoc_cache:open/2 (src/mango_idx.erl:54:21)
  dreyfus:available/0 (src/mango_idx.erl:178:14)
  dreyfus:available/0 (src/mango_native_proc.erl:312:14)
  dreyfus_bookmark:pack/1 (src/mango_cursor_text.erl:127:22)
  dreyfus_bookmark:unpack/2 (src/mango_cursor_text.erl:280:21)
  dreyfus_bookmark:update/3 (src/mango_cursor_text.erl:264:13)
  dreyfus_fabric:get_json_docs/2 (src/mango_cursor_text.erl:320:20)
  dreyfus_fabric_search:go/4 (src/mango_cursor_text.erl:159:10)
  fabric:all_docs/5 (src/mango_cursor_view.erl:218:25)
  fabric:query_view/7 (src/mango_cursor_view.erl:224:25)
  fabric:update_docs/3 (src/mango_crud.erl:37:10)
  fabric_util:is_partitioned/1 (src/mango_idx.erl:77:11)
  mochiglobal:get/1 (src/mango_util.erl:343:10)
  mochiglobal:put/2 (src/mango_util.erl:346:18)
  rexi:ping/0 (src/mango_cursor_view.erl:429:13)
  rexi:reply/1 (src/mango_cursor_view.erl:392:5)
  rexi:stream2/1 (src/mango_cursor_view.erl:387:10)
  rexi:stream_last/1 (src/mango_cursor_view.erl:389:10)
Unknown types:
  couch_att:att/0 (/Users/gaborpali/projects/github/couchdb/src/couch/include/couch_db.hrl:119:19)
 done in 0m0.46s
done (warnings were emitted)

Of course, it complains about the unknown functions. And it could even crash if I tried to add further ebins where there is something wrong.

However, this version works reasonably well (from the root of the CouchDB git clone):

$ dialyzer `find src -name ebin -depth 2`
[..]
mango_crud.erl:40:9: The pattern
          {'accepted', Results0} can never match the type
          {'aborted', _}
mango_cursor_view.erl:628:48: The call mango_cursor_view:match_and_extract_doc
         (Doc :: {[any(), ...]},
          Selector :: any(),
          any()) breaks the contract
          (Doc, Selector, Fields) -> Result
             when
                 Doc :: doc(),
                 Selector :: selector(),
                 Fields :: maybe(fields()),
                 Result :: {'match', term()} | {'no_match', 'undefined'}
[..]

Copy link
Contributor

@nickva nickva Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect. Thanks for taking the time to describe it. It would be great if one day could run these as part of CI and use type specs more.

Copy link
Contributor Author

@pgj pgj Apr 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit that it is not how make dialyze would do that. Ideally, make dialyze should be fixed to work like this. It uses rebar to create the PLT files and something in the sources makes it fail. I think that might be Elixir. Anyhow, I am a strong endorser of types and one day I will fix this somehow :-)


-type abstract_text_selector() :: {'op_and', [abstract_text_selector()]}
| {'op_or', [abstract_text_selector()]}
| {'op_not', {abstract_text_selector(), abstract_text_selector()}}
Expand All @@ -21,3 +23,13 @@
| {'op_null', {_, _}}
| {'op_default', _}
| {'op_regex', binary()}.

-type database() :: binary().
-type field() :: binary().
-type fields() :: all_fields | [field()].
-type selector() :: any().
-type ejson() :: {[{atom(), any()}]}.

-type shard_stats() :: {docs_examined, non_neg_integer()}.
-type row_property_key() :: id | key | value | doc.
-type row_properties() :: [{row_property_key(), any()}].
Loading