fix(topicidx): fix missing matches when 'a/b' and 'a/b/#' both exist

Thanks to @HJianBo for spotting this issue. The approach to fix it
is different though: we try to keep the "recurrency branch factor"
to a minimum, instead introducing new condition for the case when
filter does not match, but iteration with `ets:next/2` is not yet
finished for the prefix.

Co-Authored-By: JianBo He <heeejianbo@gmail.com>
This commit is contained in:
Andrew Mayorov 2023-08-04 16:24:13 +04:00
parent 6a13406363
commit 48a50c9137
No known key found for this signature in database
GPG Key ID: 2837C62ACFBFED5D
2 changed files with 41 additions and 20 deletions

View File

@ -59,10 +59,14 @@ match(Topic, Tab) ->
match(Words, RPrefix, Tab) -> match(Words, RPrefix, Tab) ->
Prefix = lists:reverse(RPrefix), Prefix = lists:reverse(RPrefix),
K = ets:next(Tab, {Prefix, {}}), match(ets:next(Tab, {Prefix, {}}), Prefix, Words, RPrefix, Tab).
case match_filter(Prefix, K, Words == []) of
match(K, Prefix, Words, RPrefix, Tab) ->
case match_next(Prefix, K, Words) of
true -> true ->
K; K;
skip ->
match(ets:next(Tab, K), Prefix, Words, RPrefix, Tab);
stop -> stop ->
false; false;
Matched -> Matched ->
@ -100,9 +104,11 @@ matches(Words, RPrefix, Acc, Tab) ->
matches(ets:next(Tab, {Prefix, {}}), Prefix, Words, RPrefix, Acc, Tab). matches(ets:next(Tab, {Prefix, {}}), Prefix, Words, RPrefix, Acc, Tab).
matches(K, Prefix, Words, RPrefix, Acc, Tab) -> matches(K, Prefix, Words, RPrefix, Acc, Tab) ->
case match_filter(Prefix, K, Words == []) of case match_next(Prefix, K, Words) of
true -> true ->
matches(ets:next(Tab, K), Prefix, Words, RPrefix, match_add(K, Acc), Tab); matches(ets:next(Tab, K), Prefix, Words, RPrefix, match_add(K, Acc), Tab);
skip ->
matches(ets:next(Tab, K), Prefix, Words, RPrefix, Acc, Tab);
stop -> stop ->
Acc; Acc;
Matched -> Matched ->
@ -122,29 +128,26 @@ match_add(K = {_Filter, ID}, Acc = #{}) ->
match_add(K, Acc) -> match_add(K, Acc) ->
[K | Acc]. [K | Acc].
match_filter(Prefix, {Filter, _ID}, NotPrefix) -> match_next(Prefix, {Filter, _ID}, Suffix) ->
case match_filter(Prefix, Filter) of match_filter(Prefix, Filter, Suffix);
exact -> match_next(_, '$end_of_table', _) ->
% NOTE: exact match is `true` only if we match whole topic, not prefix
NotPrefix;
Match ->
Match
end;
match_filter(_, '$end_of_table', _) ->
stop. stop.
match_filter([], []) -> match_filter([], [], []) ->
exact; true;
match_filter([], ['#']) -> match_filter([], [], _Suffix) ->
% NOTE: we matched the prefix, but there may be more matches next
skip;
match_filter([], ['#'], _Suffix) ->
% NOTE: naturally, '#' < '+', so this is already optimal for `match/2` % NOTE: naturally, '#' < '+', so this is already optimal for `match/2`
true; true;
match_filter([], ['+' | _]) -> match_filter([], ['+' | _], _Suffix) ->
plus; plus;
match_filter([], [_H | _]) -> match_filter([], [_H | _], _Suffix) ->
false; false;
match_filter([H | T1], [H | T2]) -> match_filter([H | T1], [H | T2], Suffix) ->
match_filter(T1, T2); match_filter(T1, T2, Suffix);
match_filter([H1 | _], [H2 | _]) when H2 > H1 -> match_filter([H1 | _], [H2 | _], _Suffix) when H2 > H1 ->
% NOTE: we're strictly past the prefix, no need to continue % NOTE: we're strictly past the prefix, no need to continue
stop. stop.

View File

@ -144,6 +144,24 @@ t_match_unique(_) ->
[id(M) || M <- emqx_topic_index:matches(<<"a/b/c">>, Tab, [unique])] [id(M) || M <- emqx_topic_index:matches(<<"a/b/c">>, Tab, [unique])]
). ).
t_match_wildcards(_) ->
Tab = emqx_topic_index:new(),
emqx_topic_index:insert(<<"a/b">>, id1, <<>>, Tab),
emqx_topic_index:insert(<<"a/b/#">>, id2, <<>>, Tab),
emqx_topic_index:insert(<<"a/b/#">>, id3, <<>>, Tab),
emqx_topic_index:insert(<<"a/b/c">>, id4, <<>>, Tab),
emqx_topic_index:insert(<<"a/b/+">>, id5, <<>>, Tab),
emqx_topic_index:insert(<<"a/b/d">>, id6, <<>>, Tab),
emqx_topic_index:insert(<<"a/+/+">>, id7, <<>>, Tab),
emqx_topic_index:insert(<<"a/+/#">>, id8, <<>>, Tab),
Records = [id(M) || M <- matches(<<"a/b/c">>, Tab)],
?assertEqual([id2, id3, id4, id5, id7, id8], lists:sort(Records)),
Records1 = [id(M) || M <- matches(<<"a/b">>, Tab)],
?assertEqual([id1, id2, id3, id8], lists:sort(Records1)),
ok.
match(T, Tab) -> match(T, Tab) ->
emqx_topic_index:match(T, Tab). emqx_topic_index:match(T, Tab).