Skip to content

mnesia: add select_reverse/1-4 #9475

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.

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions lib/mnesia/doc/guides/mnesia_app_b.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ limitations under the License.
write/5, delete/5, delete_object/5,
read/5, match_object/5, all_keys/4,
select/5, select/6, select_cont/3,
select_cont/4, select_reverse/5,
select_reverse/6,
index_match_object/6, index_read/6,
foldl/6, foldr/6, table_info/4,
first/3, next/4, prev/4, last/3,
Expand Down Expand Up @@ -82,6 +84,14 @@ select(ActivityId, Opaque, Tab, MatchSpec, LockKind) ->
select(ActivityId, Opaque, Tab, MatchSpec, Limit, LockKind) ->
init_select(ActivityId, Opaque, Tab, MatchSpec, Limit, LockKind).

select_reverse(ActivityId, Opaque, Tab, MatchSpec, LockKind) ->
select(ActivityId, Opaque, Tab, MatchSpec, LockKind).

select_reverse(ActivityId, Opaque, Tab, MatchSpec, Limit, LockKind) ->
select(ActivityId, Opaque, Tab, MatchSpec, Limit, LockKind).

select_cont(Tid,Ts,Cont,_Dir) -> select_cont(Tid,Ts,Cont).

select_cont(_Tid,_,{frag_cont, '$end_of_table', [],_}) -> '$end_of_table';
select_cont(Tid,Ts,{frag_cont, '$end_of_table', [{Tab,Node,Type}|Rest],Args}) ->
{Spec,LockKind,Limit} = Args,
Expand Down
9 changes: 9 additions & 0 deletions lib/mnesia/doc/guides/mnesia_chap4.md
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,8 @@ locks are acquired. The following functions are available:
equivalent of `mnesia:match_object/1`.
- [`mnesia:dirty_select(Tab, Pat)`](`mnesia:dirty_select/2`) is the dirty
equivalent of `mnesia:select/2`.
- [`mnesia:dirty_select_reverse(Tab, Pat)`](`mnesia:dirty_select_reverse/2`) is
the dirty equivalent of `mnesia:select_reverse/2`.
- [`mnesia:dirty_index_match_object(Pat, Pos)`](`mnesia:dirty_index_match_object/2`)
is the dirty equivalent of `mnesia:index_match_object/2`.
- [`mnesia:dirty_index_read(Tab, SecondaryKey, Pos)`](`mnesia:dirty_index_read/3`)
Expand Down Expand Up @@ -577,6 +579,8 @@ operations, as listed here, can be passed on as arguments to the function
- `mnesia:read/3` (`mnesia:read/1`, `mnesia:wread/1`)
- [`mnesia:match_object/2`](`mnesia:match_object/3`) (`mnesia:match_object/1`)
- [`mnesia:select/3`](`mnesia:select/2`) (`mnesia:select/2`)
- [`mnesia:select_reverse/3`](`mnesia:select_reverse/2`)
(`mnesia:select_reverse/2`)
- `mnesia:foldl/3` (`mnesia:foldl/4`, `mnesia:foldr/3`, `mnesia:foldr/4`)
- `mnesia:all_keys/1`
- `mnesia:index_match_object/4` (`mnesia:index_match_object/2`)
Expand Down Expand Up @@ -845,6 +849,11 @@ or less results than specified with `NObjects` can be returned in the result
list, even the empty list can be returned even if there are more results to
collect.

There is also [`select_reverse/1,2,3,4`](`mnesia:select_reverse/2`), should you
want to traverse from the end of the result set. This is only applicable to
`ram_copies` or `disc_copies` tables of type `ordered_set`. For other table
configurations, it behaves the same as [`mnesia:select/1,2,3,4`](`mnesia:select/2`).

> #### Warning {: .warning }
>
> There is a severe performance penalty in using `mnesia:select/1,2,3,4` after
Expand Down
226 changes: 222 additions & 4 deletions lib/mnesia/src/mnesia.erl
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,8 @@ are checked, and finally, the default value is chosen.
read/1, read/2, wread/1, read/3, read/5,
match_object/1, match_object/3, match_object/5,
select/1,select/2,select/3,select/4,select/5,select/6,
select_reverse/1,select_reverse/2,select_reverse/3,select_reverse/4,
select_reverse/5,select_reverse/6,
all_keys/1, all_keys/4,
index_match_object/2, index_match_object/4, index_match_object/6,
index_read/3, index_read/6,
Expand All @@ -315,6 +317,7 @@ are checked, and finally, the default value is chosen.
%% Dirty access regardless of activities - Read
dirty_read/1, dirty_read/2,
dirty_select/2,
dirty_select_reverse/2,
dirty_match_object/1, dirty_match_object/2, dirty_all_keys/1,
dirty_index_match_object/2, dirty_index_match_object/3,
dirty_index_read/3, dirty_slot/2,
Expand Down Expand Up @@ -374,7 +377,8 @@ are checked, and finally, the default value is chosen.
%% Module internal callback functions
raw_table_info/2, % Not for public use
remote_dirty_match_object/2, % Not for public use
remote_dirty_select/2 % Not for public use
remote_dirty_select/2, % Not for public use
remote_dirty_select_reverse/2 % Not for public use
]).

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Expand Down Expand Up @@ -2443,18 +2447,179 @@ select(Cont) ->
abort(no_transaction)
end.

% select_reverse
-doc(#{equiv => select_reverse(Tab, MatchSpec, read)}).
-doc(#{since => <<"OTP @OTP-19611@">>}).
-spec select_reverse(Tab, MatchSpec) -> [Match] when
Tab::table(), MatchSpec::ets:match_spec(), Match::term().
select_reverse(Tab, Pat) ->
select_reverse(Tab, Pat, read).
-doc(#{since => <<"OTP @OTP-19611@">>}).
-doc """
Works like `select/3`, but for table type `ordered_set`, traversing is done
starting at the last object in Erlang term order, and moves to the first. For
all other table types, the return value is identical to that of `select/3`.

See `select/3` for more information.
""".
-spec select_reverse(Tab, Spec, LockKind) -> [Match] when
Tab::table(), Spec::ets:match_spec(),
Match::term(),LockKind::lock_kind().
select_reverse(Tab, Pat, LockKind)
when is_atom(Tab), Tab /= schema, is_list(Pat) ->
case get(mnesia_activity_state) of
{?DEFAULT_ACCESS, Tid, Ts} ->
select_reverse(Tid, Ts, Tab, Pat, LockKind);
{Mod, Tid, Ts} ->
Mod:select_reverse(Tid, Ts, Tab, Pat, LockKind);
_ ->
abort(no_transaction)
end;
select_reverse(Tab, Pat, _Lock) ->
abort({badarg, Tab, Pat}).

-doc false.
select_reverse(Tid, Ts, Tab, Spec, LockKind) ->
SelectFun = fun(FixedSpec) -> dirty_select_reverse(Tab, FixedSpec) end,
fun_select_reverse(Tid, Ts, Tab, Spec, LockKind, Tab, SelectFun).

-doc false.
fun_select_reverse(Tid, Ts, Tab, Spec, LockKind, TabPat, SelectFun) ->
case element(1, Tid) of
ets ->
mnesia_lib:db_select_rev(ram_copies, Tab, Spec);
tid ->
select_lock(Tid,Ts,LockKind,Spec,Tab),
Store = Ts#tidstore.store,
Written = ?ets_match_object(Store, {{TabPat, '_'}, '_', '_'}),
case Written of
[] ->
%% Nothing changed in the table during this transaction,
%% Simple case get results from [d]ets
SelectFun(Spec);
_ ->
%% Hard (slow case) records added or deleted earlier
%% in the transaction, have to cope with that.
Type = val({Tab, setorbag}),
FixedSpec = get_record_pattern(Spec),
TabRecs = SelectFun(FixedSpec),
FixedRes = add_match(Written, TabRecs, Type),
CMS = ets:match_spec_compile(Spec),
ets:match_spec_run(FixedRes, CMS)
end;
_Protocol ->
SelectFun(Spec)
end.

%% Breakable Select Reverse
-doc(#{since => <<"OTP @OTP-19611@">>}).
-doc """
Select the objects in `Tab` against `MatchSpec` in reverse order.

Matches the objects in table `Tab` using a `match_spec` as described in the
[ERTS](`e:erts:index.html`) User's Guide, and returns a chunk of terms and a
continuation. The wanted number of returned terms is specified by argument
`NObjects`. The lock argument can be `read` or `write`. The continuation is to
be used as argument to `mnesia:select_reverse/1`, if more or all answers are needed.

Notice that for best performance, `select_reverse` is to be used before any modifying
operations are done on that table in the same transaction. That is, do not use
`mnesia:write` or `mnesia:delete` before a `mnesia:select_reverse`. For efficiency,
`NObjects` is a recommendation only and the result can contain anything from an
empty list to all available results.
""".
-spec select_reverse(Tab, Spec, N, LockKind) -> {[Match], Cont} | '$end_of_table' when
Tab::table(), Spec::ets:match_spec(),
Match::term(), N::non_neg_integer(),
LockKind::lock_kind(),
Cont::select_continuation().
select_reverse(Tab, Pat, NObjects, LockKind)
when is_atom(Tab), Tab /= schema, is_list(Pat), is_integer(NObjects) ->
case get(mnesia_activity_state) of
{?DEFAULT_ACCESS, Tid, Ts} ->
select_reverse(Tid, Ts, Tab, Pat, NObjects, LockKind);
{Mod, Tid, Ts} ->
Mod:select_reverse(Tid, Ts, Tab, Pat, NObjects, LockKind);
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this line is still not covered.

Copy link
Author

Choose a reason for hiding this comment

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

This (and the other coverage comments below) appear to be covered for me running mnesia_evil_coverage_test + mnesia_frag_test. Can you share the test command you're running that shows the missing coverage in these spots?

for reference, what I'm doing is temporarily adding {mnesia_frag_test, all}, to the all/0 list in mnesia_evil_coverage_test.erl, then I run the evil tests with

make mnesia_test ARGS="-mnesia dir '"/mnesia_test_tmp"' -logdir /mnesia_test_tmp -suite mnesia_SUITE -group evil -cover $ERL_TOP/lib/mnesia/test/mnesia.cover"

(the -mnesia dir and -logdir probably aren't relevant, but just including for completeness)

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I will check then, maybe some test that covers this is disabled or something.
The command I'm using is:
make mnesia_test ARGS="-cover /workspaces/otp/lib/mnesia/test/mnesia.cover"

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, I've checked this, and there are some tests which should be running, but they're not for some reason.
I will try to fix this, but we don't have to wait for the fix with this PR.

I will check this PR again, and also run this through our CI, and let you know if it's okay to merge.

Copy link
Author

Choose a reason for hiding this comment

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

Ahh I see, sounds good, happy to rebase if you happen to fix the tests before then

_ ->
abort(no_transaction)
end;
select_reverse(Tab, Pat, NObjects, _Lock) ->
abort({badarg, Tab, Pat, NObjects}).

-doc false.
select_reverse(Tid, Ts, Tab, Spec, NObjects, LockKind) ->
Where = val({Tab,where_to_read}),
Type = mnesia_lib:storage_type_at_node(Where,Tab),
InitFun = fun(FixedSpec) -> dirty_sel_init(Where,Tab,FixedSpec,NObjects,Type,reverse) end,
fun_select_reverse(Tid,Ts,Tab,Spec,LockKind,Tab,InitFun,NObjects,Where,Type).

-doc false.
fun_select_reverse(Tid, Ts, Tab, Spec, LockKind, TabPat, Init, NObjects, Node, Storage) ->
Def = #mnesia_select{tid=Tid,node=Node,storage=Storage,tab=Tab,orig=Spec},
case element(1, Tid) of
ets ->
select_state(mnesia_lib:db_select_rev_init(ram_copies,Tab,Spec,NObjects),Def);
tid ->
select_lock(Tid,Ts,LockKind,Spec,Tab),
Store = Ts#tidstore.store,
do_fixtable(Tab, Store),

Written0 = ?ets_match_object(Store, {{TabPat, '_'}, '_', '_'}),
case Written0 of
[] ->
%% Nothing changed in the table during this transaction,
%% Simple case get results from [d]ets
select_state(Init(Spec),Def);
_ ->
%% Hard (slow case) records added or deleted earlier
%% in the transaction, have to cope with that.
Type = val({Tab, setorbag}),
Written =
if Type == ordered_set -> %% Sort stable, in descending order
lists:sort(fun(A, B) -> element(1, A) > element(1, B) end, Written0);
true ->
Written0
end,
FixedSpec = get_record_pattern(Spec),
CMS = ets:match_spec_compile(Spec),
trans_select(Init(FixedSpec),
Def#mnesia_select{written=Written,spec=CMS,type=Type, orig=FixedSpec})
end;
_Protocol ->
select_state(Init(Spec),Def)
end.

-doc(#{since => <<"OTP @OTP-19611@">>}).
-doc """
Continue selecting objects.

Selects more objects with the match specification initiated by
`mnesia:select_reverse/4`.

Notice that any modifying operations, that is, `mnesia:write` or
`mnesia:delete`, that are done between the `mnesia:select_reverse/4` and
`mnesia:select_reverse/1` calls are not visible in the result.
""".
-spec select_reverse(Cont) -> {[Match], Cont} | '$end_of_table' when
Match::term(),
Cont::select_continuation().
select_reverse(Cont) ->
select(Cont).

-doc false.
select_cont(_Tid,_Ts,'$end_of_table') ->
'$end_of_table';
select_cont(Tid,_Ts,State=#mnesia_select{tid=Tid,cont=Cont, orig=Ms})
when element(1,Tid) == ets ->
case Cont of
'$end_of_table' -> '$end_of_table';
_ -> select_state(mnesia_lib:db_select_cont(ram_copies,Cont,Ms),State)
_ ->
Result = mnesia_lib:db_select_cont(ram_copies,Cont,Ms),
select_state(Result,State)
end;
select_cont(Tid,_,State=#mnesia_select{tid=Tid,written=[]}) ->
select_state(dirty_sel_cont(State),State);
select_cont(Tid,_Ts,State=#mnesia_select{tid=Tid}) ->
select_cont(Tid,_Ts,State=#mnesia_select{tid=Tid}) ->
trans_select(dirty_sel_cont(State), State);
select_cont(Tid2,_,#mnesia_select{tid=_Tid1})
when element(1,Tid2) == tid -> % Mismatching tids
Expand Down Expand Up @@ -2883,9 +3048,62 @@ remote_dirty_select(Tab, [{HeadPat,_, _}] = Spec, [Pos | Tail])
remote_dirty_select(Tab, Spec, _) ->
mnesia_lib:db_select(Tab, Spec).

-doc(#{since => <<"OTP @OTP-19611@">>}).
-doc """
Dirty equivalent to `mnesia:select_reverse/2`.
""".
-spec dirty_select_reverse(Tab, Spec) -> [Match] when
Tab::table(), Spec::ets:match_spec(), Match::term().
dirty_select_reverse(Tab, Spec) when is_atom(Tab), Tab /= schema, is_list(Spec) ->
dirty_rpc(Tab, ?MODULE, remote_dirty_select_reverse, [Tab, Spec]);
dirty_select_reverse(Tab, Spec) ->
abort({bad_type, Tab, Spec}).

-doc false.
remote_dirty_select_reverse(Tab, Spec) ->
case Spec of
[{HeadPat, _, _}] when is_tuple(HeadPat), tuple_size(HeadPat) > 2 ->
Key = element(2, HeadPat),
case has_var(Key) of
false ->
mnesia_lib:db_select_rev(Tab, Spec);
true ->
PosList = regular_indexes(Tab),
remote_dirty_select_reverse(Tab, Spec, PosList)
end;
_ ->
mnesia_lib:db_select_rev(Tab, Spec)
end.

remote_dirty_select_reverse(Tab, [{HeadPat,_, _}] = Spec, [Pos | Tail])
when is_tuple(HeadPat), tuple_size(HeadPat) > 2, Pos =< tuple_size(HeadPat) ->
Key = element(Pos, HeadPat),
case has_var(Key) of
false ->
Recs = mnesia_index:dirty_select(Tab, HeadPat, Pos),
%% Returns the records without applying the match spec
%% The actual filtering is handled by the caller
CMS = ets:match_spec_compile(Spec),
case val({Tab, setorbag}) of
ordered_set ->
DescFun = fun(A, B) -> A > B end,
ets:match_spec_run(lists:sort(DescFun, Recs), CMS);
_ ->
ets:match_spec_run(Recs, CMS)
end;
true ->
remote_dirty_select_reverse(Tab, Spec, Tail)
end;
remote_dirty_select_reverse(Tab, Spec, _) ->
mnesia_lib:db_select_rev(Tab, Spec).

-doc false.
dirty_sel_init(Node,Tab,Spec,NObjects,Type) ->
do_dirty_rpc(Tab,Node,mnesia_lib,db_select_init,[Type,Tab,Spec,NObjects]).
dirty_sel_init(Node,Tab,Spec,NObjects,Type,forward).
dirty_sel_init(Node,Tab,Spec,NObjects,Type,forward) ->
do_dirty_rpc(Tab,Node,mnesia_lib,db_select_init,[Type,Tab,Spec,NObjects]);
dirty_sel_init(Node,Tab,Spec,NObjects,Type,reverse) ->
do_dirty_rpc(Tab,Node,mnesia_lib,db_select_rev_init,[Type,Tab,Spec,NObjects]).

dirty_sel_cont(#mnesia_select{cont='$end_of_table'}) -> '$end_of_table';
dirty_sel_cont(#mnesia_select{node=Node,tab=Tab,storage=Type,cont=Cont,orig=Ms}) ->
Expand Down
9 changes: 9 additions & 0 deletions lib/mnesia/src/mnesia_frag.erl
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
write/5, delete/5, delete_object/5,
read/5, match_object/5, all_keys/4,
select/5,select/6,select_cont/3,
select_cont/4, select_reverse/5,
select_reverse/6,
index_match_object/6, index_read/6,
foldl/6, foldr/6, table_info/4,
first/3, next/4, prev/4, last/3,
Expand Down Expand Up @@ -118,6 +120,11 @@ select(ActivityId, Opaque, Tab, MatchSpec, LockKind) ->
select(ActivityId, Opaque, Tab, MatchSpec, Limit, LockKind) ->
init_select(ActivityId, Opaque, Tab, MatchSpec, Limit, LockKind).

select_reverse(ActivityId, Opaque, Tab, MatchSpec, LockKind) ->
select(ActivityId, Opaque, Tab, MatchSpec, LockKind).

select_reverse(ActivityId, Opaque, Tab, MatchSpec, Limit, LockKind) ->
select(ActivityId, Opaque, Tab, MatchSpec, Limit, LockKind).

all_keys(ActivityId, Opaque, Tab, LockKind) ->
Match = [mnesia:all_keys(ActivityId, Opaque, Frag, LockKind)
Expand Down Expand Up @@ -336,6 +343,8 @@ init_select(Tid,Opaque,Tab,Pat,Limit,LockKind) ->
frag_sel_cont(Res, NameNodes, {Pat,LockKind,Limit})
end.

select_cont(Tid,Ts,Cont,_Dir) -> select_cont(Tid,Ts,Cont).

select_cont(_Tid,_,{frag_cont, '$end_of_table', [],_}) -> '$end_of_table';
select_cont(Tid,Ts,{frag_cont, '$end_of_table', [{Tab,Node,Type}|Rest],Args}) ->
{Spec,LockKind,Limit} = Args,
Expand Down
Loading
Loading