Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Dialyzer fixes #96

Merged
merged 14 commits into from over 2 years ago

2 participants

Ryan Zezeski Russell Brown
Ryan Zezeski
Collaborator

No description provided.

rzezeski added some commits
Ryan Zezeski rzezeski Remove old entries from ignore file 68a3715
Ryan Zezeski rzezeski Allow multiline queries
A multiline query cases a badmatch because the parser assumes single line
queries.  Changing the pattern and testing a few multiline queries showed they
are parsed just the same as their single line counterparts.
7784f10
Ryan Zezeski rzezeski Use canonical riak_object type
This fixes some dialyzer errors that, frankly, confused me very much.
3eea7dd
Ryan Zezeski rzezeski Fix spec for make_indexed_doc ac674d9
Ryan Zezeski rzezeski Use more specific type for idxdoc 8466770
Ryan Zezeski rzezeski Remove unreachable code
Since a list is always passed to this function the first function head can
never match.
0ab4951
Ryan Zezeski rzezeski Make dialyzer happy
Dialyzer was giving me the following warning so I refactored into a map call
which I think looks cleaner anyways.

    The pattern [{{Idx, Node}, 'primary'} | _] can never match the type []
226146f
Ryan Zezeski rzezeski Add canonical index type
Add a canonical index type that all of riak_search, as well as it's clients,
can reference.
f44c92a
Ryan Zezeski rzezeski Add specs to Search/Merge Index boundary 458a073
Ryan Zezeski rzezeski Remove unused macros 38e6643
Ryan Zezeski rzezeski Move spec next to function 3d35ace
Ryan Zezeski rzezeski Spec out the query stream code
Gave a good first round spec-out of the query streaming code.  Some of the specs
could be tighter.  All good things come with time.
b8b7c8c
Ryan Zezeski rzezeski Fix spec for index_object e64c4fc
Ryan Zezeski rzezeski Remove unreachable code
I'm not sure how it ever got there but this code cannot be reached as the
string/1 fun will always return either `{ok, Tokens, Line}` or
`{error, Error, Line}`.
7818ac9
Russell Brown

:trollface: I saw a meaningless micro-benchmark on the erlang ML that says list comprehensions are much faster, and Kostis said the same when he talked about his refactoring tool at EF London :trollface:

lists:map (in this case) is easier to read.

Collaborator

Interesting, yea I remember andrew telling me list comprehensions are the fastest method of iterating list. I just gave up because dialyzer was trolling me. I haven't seen this fun dominate any of the fprof analysis I've done yet but I'll remember to keep your note in mind.

Russell Brown

A quick grep seems to point at this only being called with a list…did you refactor out calls to it with an array, or was it old, dead code?

Collaborator

Yes, it is dead code.

Russell Brown
Collaborator

I still get some dialyzer warnings, but the changes all look good, and search still passes tests. +1 to merge.

Russell Brown
Collaborator

Those warnings

Proceeding with analysis...
merge_index_backend.erl:151: Type specification merge_index_backend:stream_to(itr_result(),sender()) -> any() is a       supertype of the success typing:  merge_index_backend:stream_to('eof' | {_,_},'ignore' | {'fsm','undefined' |   reference(),pid()} | {'raw',reference(),pid()}) -> any()
riak_search_config.erl:51: Type specification riak_search_config:get_schema(any()) -> {'ok',any()} | {'error','badarg'} | any() is a supertype of the success typing: riak_search_config:get_schema(atom() | binary() | maybe_improper_list(binary()  | maybe_improper_list(any(),binary() | []) | byte(),binary() | []) | integer() | tuple()) -> any()
riak_search_op_term.erl:60: Type specification riak_search_op_term:start_loop(any(),pid(),reference(),#search_state{}) -> any() is a supertype of the success typing: riak_search_op_term:start_loop(#term{},pid(),reference(),#search_state{}) ->  {'disconnect',_} | {'error',_}
riak_search_utils.erl:116: Type specification riak_search_utils:choose([any()]) -> any() is a supertype of the success typing: riak_search_utils:choose([any(),...]) -> any()
riak_search_utils.erl:176: The variable Error can never match since previous clauses completely covered the type {'value',_,[{_,_}]}
Ryan Zezeski rzezeski merged commit 7818ac9 into from
Ryan Zezeski rzezeski closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 14 unique commits by 1 author.

Mar 03, 2012
Ryan Zezeski rzezeski Remove old entries from ignore file 68a3715
Ryan Zezeski rzezeski Allow multiline queries
A multiline query cases a badmatch because the parser assumes single line
queries.  Changing the pattern and testing a few multiline queries showed they
are parsed just the same as their single line counterparts.
7784f10
Ryan Zezeski rzezeski Use canonical riak_object type
This fixes some dialyzer errors that, frankly, confused me very much.
3eea7dd
Ryan Zezeski rzezeski Fix spec for make_indexed_doc ac674d9
Ryan Zezeski rzezeski Use more specific type for idxdoc 8466770
Ryan Zezeski rzezeski Remove unreachable code
Since a list is always passed to this function the first function head can
never match.
0ab4951
Ryan Zezeski rzezeski Make dialyzer happy
Dialyzer was giving me the following warning so I refactored into a map call
which I think looks cleaner anyways.

    The pattern [{{Idx, Node}, 'primary'} | _] can never match the type []
226146f
Ryan Zezeski rzezeski Add canonical index type
Add a canonical index type that all of riak_search, as well as it's clients,
can reference.
f44c92a
Ryan Zezeski rzezeski Add specs to Search/Merge Index boundary 458a073
Ryan Zezeski rzezeski Remove unused macros 38e6643
Ryan Zezeski rzezeski Move spec next to function 3d35ace
Ryan Zezeski rzezeski Spec out the query stream code
Gave a good first round spec-out of the query streaming code.  Some of the specs
could be tighter.  All good things come with time.
b8b7c8c
Ryan Zezeski rzezeski Fix spec for index_object e64c4fc
Ryan Zezeski rzezeski Remove unreachable code
I'm not sure how it ever got there but this code cannot be reached as the
string/1 fun will always return either `{ok, Tokens, Line}` or
`{error, Error, Line}`.
7818ac9
This page is out of date. Refresh to see the latest.
7 .gitignore
... ... @@ -1,13 +1,6 @@
1 1 .eunit/
2 2 deps/*
3 3 priv/*
4   -*.o
5   -*.beam
6   -*.jar
7   -apps/qilr/include/analysis_pb.hrl
8   -apps/qilr/src/qilr_parse.erl
9   -apps/qilr/src/qilr_scan.erl
10   -build
11 4 ebin
12 5 doc/*.html
13 6 doc/edoc-info
9 apps/lucene_parser/src/lucene_parser.erl
@@ -12,7 +12,7 @@
12 12 %% consists of a nested set of Erlang records found in lucene_parser.hrl.
13 13 parse(DefaultIndex, DefaultField, QueryString) when is_list(DefaultIndex) andalso is_list(DefaultField) andalso is_list(QueryString) ->
14 14 case lucene_scan:string(QueryString) of
15   - {ok, Tokens, 1} ->
  15 + {ok, Tokens, _Line} ->
16 16 case lucene_parse:parse(Tokens) of
17 17 {ok, QueryGraph} ->
18 18 %% Success case.
@@ -25,12 +25,7 @@ parse(DefaultIndex, DefaultField, QueryString) when is_list(DefaultIndex) andals
25 25 %% Other parsing error.
26 26 {error, Error}
27 27 end;
28   - {error, Error} ->
29   - %% Scanning error.
30   - {error, Error};
31   - {error, Error, _} ->
32   - %% Scanning error.
33   - {error, Error}
  28 + {error, Error, _Line} -> {error, Error}
34 29 end.
35 30
36 31
21 include/riak_search.hrl
... ... @@ -1,13 +1,28 @@
  1 +%%%===================================================================
  2 +%%% Macros
  3 +%%%===================================================================
  4 +
1 5 -ifndef(PRINT).
2 6 -define(PRINT(Var), io:format("DEBUG: ~p:~p - ~p~n~n ~p~n~n", [?MODULE, ?LINE, ??Var, Var])).
3   --define(TIMEON, erlang:put(debug_timer, [now()|case erlang:get(debug_timer) == undefined of true -> []; false -> erlang:get(debug_timer) end])).
4   --define(TIMEOFF(Var), io:format("~s :: ~10.2f ms : ~p : ~p~n", [string:copies(" ", length(erlang:get(debug_timer))), (timer:now_diff(now(), hd(erlang:get(debug_timer)))/1000), ??Var, Var]), erlang:put(debug_timer, tl(erlang:get(debug_timer)))).
5 7 -endif.
6 8 -define(FMT(S, L), lists:flatten(io_lib:format(S, L))).
7 9
8 10 -define(DEFAULT_INDEX, <<"search">>).
9 11 -define(RESULTVEC_SIZE, 1000).
10   --define(OPKEY(Tag, Op), {Tag, element(2, Op)}).
  12 +
  13 +%%%===================================================================
  14 +%%% Types
  15 +%%%===================================================================
  16 +
  17 +-type index() :: binary().
  18 +-type field() :: binary().
  19 +-type s_term() :: binary().
  20 +
  21 +-type stream_ref() :: {stream_response, reference()}.
  22 +
  23 +%%%===================================================================
  24 +%%% Records
  25 +%%%===================================================================
11 26
12 27 -record(search_state, {
13 28 parent=undefined,
35 src/merge_index_backend.erl
@@ -23,8 +23,17 @@
23 23 -export([stream_worker/6,
24 24 range_worker/8]).
25 25
  26 +-include_lib("riak_core/include/riak_core_vnode.hrl").
26 27 -include("riak_search.hrl").
27 28
  29 +-type mi_size() :: all | pos_integer().
  30 +-type(iterator() :: fun(() -> {any(), iterator()}
  31 + | eof
  32 + | {error, Reason::any()})).
  33 +-type itr_result() :: {any(), iterator()}
  34 + | eof
  35 + | {error, Reason::any()}.
  36 +
28 37 % @type state() = term().
29 38 -record(state, {partition, pid}).
30 39
@@ -65,6 +74,7 @@ info(Index, Field, Term, Sender, State) ->
65 74 riak_search_backend:info_response(Sender, [{Term, node(), Count}]),
66 75 noreply.
67 76
  77 +-spec stream(index(), field(), s_term(), fun(), sender(), #state{}) -> noreply.
68 78 stream(Index, Field, Term, Filter, Sender, State) ->
69 79 Pid = State#state.pid,
70 80 spawn_link(?MODULE, stream_worker, [Pid, Index, Field,
@@ -125,20 +135,25 @@ drop(State) ->
125 135 %%% Internal Functions
126 136 %%%===================================================================
127 137
  138 +-spec stream_worker(pid(), index(), field(), s_term(), fun(), sender()) ->
  139 + any().
128 140 stream_worker(Pid, Index, Field, Term, Filter, Sender) ->
129 141 Iter = merge_index:lookup(Pid, Index, Field, Term, Filter),
130   - iterate(Iter(), Sender).
  142 + stream_to(Iter(), Sender).
131 143
  144 +-spec range_worker(pid(), index(), field(), s_term(), s_term(), mi_size(),
  145 + fun(), sender()) -> any().
132 146 range_worker(Pid, Index, Field, StartTerm, EndTerm, Size, Filter, Sender) ->
133 147 Iter = merge_index:range(Pid, Index, Field, StartTerm, EndTerm,
134 148 Size, Filter),
135   - iterate(Iter(), Sender).
136   -
137   -iterate(eof, Sender) ->
138   - riak_search_backend:response_done(Sender);
139   -iterate({error, Reason}, Sender) ->
140   - riak_search_backend:response_error(Sender, Reason);
141   -iterate({Results, Iter}, Sender) ->
142   - riak_search_backend:response_results(Sender, Results),
143   - iterate(Iter(), Sender).
  149 + stream_to(Iter(), Sender).
  150 +
  151 +-spec stream_to(itr_result(), sender()) -> any().
  152 +stream_to(eof, To) ->
  153 + riak_search_backend:response_done(To);
  154 +stream_to({error, Reason}, To) ->
  155 + riak_search_backend:response_error(To, Reason);
  156 +stream_to({Results, Iter}, To) ->
  157 + riak_search_backend:response_results(To, Results),
  158 + stream_to(Iter(), To).
144 159
3  src/riak_search_config.erl
@@ -46,6 +46,9 @@ clear() ->
46 46
47 47 %% Get schema information for the provided index name.
48 48 %% @param Schema - Either the name of an index, or a schema record.
  49 +%%
  50 +%% TODO This spec/fun needs help
  51 +-spec get_schema(any()) -> {ok, any()} | {error, badarg} | any().
49 52 get_schema(Schema) when is_tuple(Schema) ->
50 53 case element(1, Schema) of
51 54 riak_search_schema ->
23 src/riak_search_kv_hook.erl
@@ -32,14 +32,13 @@
32 32 -type fargs() :: {function(), args()}.
33 33 -type extractdef() :: mfargs() | fargs().
34 34
35   --type riak_object() :: tuple(). % no good way to define riak_object
  35 +-type obj() :: riak_object:riak_object().
36 36
37   --type extract_qfun() :: fun((riak_object(),any()) -> search_fields()).
  37 +-type extract_qfun() :: fun((obj(),any()) -> search_fields()).
38 38 -type args() :: any().
39 39
40   --type index() :: binary().
41 40 -type docid() :: binary().
42   --type idxdoc() :: tuple(). % #riak_indexed_doc{}
  41 +-type idxdoc() :: #riak_idx_doc{}.
43 42
44 43 -type search_fields() :: [{search_field(),search_data()}].
45 44 -type search_field() :: string().
@@ -100,8 +99,7 @@ precommit_def() ->
100 99 %% Precommit hook for riak k/v and search integration. Executes
101 100 %% the desired mapping on the riak object to produce a search
102 101 %% document to store in riak search.
103   -%%
104   --spec precommit(riak_object()) -> {fail, any()} | riak_object().
  102 +-spec precommit(obj()) -> {fail, any()} | obj().
105 103 precommit(RiakObject) ->
106 104 Extractor = get_extractor(RiakObject),
107 105 try
@@ -118,7 +116,7 @@ precommit(RiakObject) ->
118 116
119 117 %% Decide if an object should be indexed, and if so the extraction function to
120 118 %% pull out the search fields.
121   --spec get_extractor(riak_object()) -> extractdef().
  119 +-spec get_extractor(obj()) -> extractdef().
122 120 get_extractor(RiakObject) ->
123 121 BucketProps = riak_core_bucket:get_bucket(riak_object:bucket(RiakObject)),
124 122 Ex = try_keys([?LEGACY_EX_PROP_NAME, ?EX_PROP_NAME], BucketProps),
@@ -172,7 +170,7 @@ to_modfun(Val) ->
172 170 %%
173 171 %% Index the provided riak object and return ok on success.
174 172 %%
175   --spec index_object(riak_object(), extractdef()) -> ok.
  173 +-spec index_object(obj(), extractdef()) -> ok | {error, Reason::any()}.
176 174 index_object(RiakObject, Extractor) ->
177 175 %% Set up
178 176 {ok, RiakClient} = riak:local_client(),
@@ -197,7 +195,8 @@ index_object(RiakObject, Extractor) ->
197 195 end.
198 196
199 197 %% Make an indexed document under Index/DocId from the RiakObject
200   --spec make_indexed_doc(index(), docid(), riak_object(), extractdef()) -> idxdoc().
  198 +-spec make_indexed_doc(index(), docid(), obj(), extractdef()) ->
  199 + idxdoc() | deleted.
201 200 make_indexed_doc(Index, DocId, RiakObject, Extractor) ->
202 201 case riak_kv_util:is_x_deleted(RiakObject) of
203 202 true ->
@@ -211,17 +210,17 @@ make_indexed_doc(Index, DocId, RiakObject, Extractor) ->
211 210 IdxDoc
212 211 end.
213 212
214   --spec make_index(riak_object()) -> binary().
  213 +-spec make_index(obj()) -> binary().
215 214 make_index(RiakObject) ->
216 215 riak_object:bucket(RiakObject).
217 216
218   --spec make_docid(riak_object()) -> binary().
  217 +-spec make_docid(obj()) -> binary().
219 218 make_docid(RiakObject) ->
220 219 riak_object:key(RiakObject).
221 220
222 221 %% Run the extraction function against the RiakObject to get a list of
223 222 %% search fields and data
224   --spec run_extract(riak_object(), string(), extractdef()) -> search_fields().
  223 +-spec run_extract(obj(), string(), extractdef()) -> search_fields().
225 224 run_extract(RiakObject, DefaultField, {M, F, A}) ->
226 225 M:F(RiakObject, DefaultField, A);
227 226 run_extract(RiakObject, DefaultField, {F, A}) ->
2  src/riak_search_op_proximity.erl
@@ -213,8 +213,6 @@ get_position_heads([], Acc) ->
213 213 lists:reverse(Acc).
214 214
215 215 %% Given a list of integers, return the smallest integer that is a duplicate.
216   -get_smallest_duplicate_or_not(undefined) ->
217   - undefined;
218 216 get_smallest_duplicate_or_not(List) ->
219 217 get_smallest_duplicate_or_not(lists:sort(List), undefined, undefined).
220 218 get_smallest_duplicate_or_not([H,H|Rest], SmallestDup, SmallestVal)
12 src/riak_search_op_term.erl
@@ -57,6 +57,7 @@ chain_op(Op, OutputPid, OutputRef, State) ->
57 57 erlang:spawn_link(F),
58 58 {ok, 1}.
59 59
  60 +-spec start_loop(any(), pid(), reference(), #search_state{}) -> any().
60 61 start_loop(Op, OutputPid, OutputRef, State) ->
61 62 %% Get the current index/field...
62 63 IndexName = State#search_state.index,
@@ -71,6 +72,7 @@ start_loop(Op, OutputPid, OutputRef, State) ->
71 72 TransformFun = generate_transform_function(Op, State),
72 73 riak_search_op_utils:gather_stream_results(Ref, OutputPid, OutputRef, TransformFun).
73 74
  75 +-spec stream(index(), field(), s_term(), fun()) -> {ok, stream_ref()}.
74 76 stream(Index, Field, Term, FilterFun) ->
75 77 %% Get the primary preflist, minus any down nodes. (We don't use
76 78 %% secondary nodes since we ultimately read results from one node
@@ -88,7 +90,8 @@ stream(Index, Field, Term, FilterFun) ->
88 90 PreflistEntry ->
89 91 PreflistEntry = PreflistEntry
90 92 end,
91   - riak_search_vnode:stream([PreflistEntry], Index, Field, Term, FilterFun, self()).
  93 + riak_search_vnode:stream([PreflistEntry], Index, Field, Term, FilterFun,
  94 + self()).
92 95
93 96 default_filter(_, _) -> true.
94 97
@@ -147,8 +150,7 @@ calculate_score(ScoringVars, Props) ->
147 150 end,
148 151 lists:keystore(score, 1, Props, {score, ScoreList}).
149 152
  153 +-spec get_preflist(binary(), pos_integer()) -> list().
150 154 get_preflist(DocIdx, NVal) ->
151   - [{Idx,Node}
152   - || {{Idx,Node},primary} <- riak_core_apl:get_primary_apl(DocIdx,
153   - NVal,
154   - riak_search)].
  155 + lists:map(fun({IdxNode, _}) -> IdxNode end,
  156 + riak_core_apl:get_primary_apl(DocIdx, NVal, riak_search)).
4 src/riak_search_op_utils.erl
@@ -147,6 +147,10 @@ gather_iterator_results(OutputPid, OutputRef, {eof, _}, Acc) ->
147 147 %% Gathers result vectors sent to the current Pid by a backend stream
148 148 %% or range operation, run a transform function, and shuttles the
149 149 %% results to the given OutputPid and OutputRef.
  150 +%%
  151 +%% TODO maybe instead of throw return timeout or {error, timeout}
  152 +-spec gather_stream_results(stream_ref(), pid(), reference(), fun()) ->
  153 + any() | no_return().
150 154 gather_stream_results(Ref, OutputPid, OutputRef, TransformFun) ->
151 155 receive
152 156 {Ref, done} ->
5 src/riak_search_ring_utils.erl
@@ -5,6 +5,8 @@
5 5 calc_partition/3
6 6 ]).
7 7
  8 +-include("riak_search.hrl").
  9 +
8 10 %% Pick out the preflist of covering nodes. There are two approaches
9 11 %% in the face of down nodes. One is to minimize the amount of
10 12 %% duplicate data that we read. The other is maximize load
@@ -146,7 +148,6 @@ zip_with_partition_and_index(Postings) ->
146 148 %% riak_core_util:chash_key/N, but we don't allow Riak Search to
147 149 %% specify custom hashes. It just takes too long to look up for every
148 150 %% term and kills performance.
  151 +-spec calc_partition(index(), field(), term()) -> binary().
149 152 calc_partition(Index, Field, Term) ->
150 153 crypto:sha(term_to_binary({Index, Field, Term})).
151   -
152   -
11 src/riak_search_utils.erl
@@ -112,15 +112,12 @@ current_key_clock() ->
112 112 (Seconds * 1000000) +
113 113 MilliSeconds.
114 114
115   -%% Choose a random element from the List or Array.
116   -choose(List) when is_list(List) ->
  115 +%% Choose a random element from the List.
  116 +-spec choose(list()) -> any().
  117 +choose(List) ->
117 118 random:seed(now()),
118 119 N = random:uniform(length(List)),
119   - lists:nth(N, List);
120   -choose(Array) when element(1, Array) == array ->
121   - random:seed(now()),
122   - N = random:uniform(Array:size()),
123   - Array:get(N - 1).
  120 + lists:nth(N, List).
124 121
125 122 %% Take the first defined element.
126 123 coalesce(undefined, B) -> B;
5 src/riak_search_vnode.erl
@@ -19,8 +19,7 @@
19 19
20 20 -include_lib("riak_core/include/riak_core_vnode.hrl").
21 21 -include_lib("riak_core/include/riak_core_pb.hrl").
22   --define(PRINT(Var), io:format("DEBUG: ~p:~p - ~p~n~n ~p~n~n", [?MODULE, ?LINE, ??Var, Var])).
23   -
  22 +-include("riak_search.hrl").
24 23
25 24 -record(vstate, {idx, bmod, bstate}).
26 25 -record(index_v1, {iftvp_list}).
@@ -53,6 +52,8 @@ info(Preflist, Index, Field, Term, ReplyTo) ->
53 52 command(Preflist, Req, {raw, Ref, ReplyTo}),
54 53 {ok, Ref}.
55 54
  55 +-spec stream(list(), index(), field(), term(), fun(), pid()) ->
  56 + {ok, stream_ref()}.
56 57 stream(Preflist, Index, Field, Term, FilterFun, ReplyTo) ->
57 58 Req = #stream_v1{
58 59 index = Index,

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.