Skip to content

Commit

Permalink
z_search: Use joins for search while the category tree is renumbering
Browse files Browse the repository at this point in the history
Although using joins is considerably slower, this way, the search
results are still correct when the database is updating itself.

Added public function m_category:is_tree_dirty/1 which can be used to
check if the current category tree is dirty or now.

Fixes #308
  • Loading branch information
Arjan Scherpenisse committed Dec 10, 2012
1 parent 8788a42 commit eb0ae0a
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 38 deletions.
23 changes: 20 additions & 3 deletions src/models/m_category.erl
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@
renumber/1,
renumber_pivot_task/1,
enumerate/1,
boundaries/2
boundaries/2,
is_tree_dirty/1
]).


Expand Down Expand Up @@ -668,6 +669,7 @@ renumber_transaction(Context) ->
, Context)
|| {CatId, Nr, Level, Left, Right, Path} <- Enums
],
set_tree_dirty(true, Context),
z_pivot_rsc:insert_task_after(10, ?MODULE, renumber_pivot_task, "m_category:renumber", [], Context),
ok.

Expand All @@ -678,9 +680,11 @@ renumber_pivot_task(Context) ->
where c.id = r.category_id
and (r.pivot_category_nr is null or r.pivot_category_nr <> c.nr)
order by r.id
limit 500", Context),
limit 1000", Context),
case Nrs of
[] ->
?zInfo("Category renumbering completed.", Context),
set_tree_dirty(false, Context),
ok;
Ids ->
ok = z_db:transaction(fun(Ctx) ->
Expand All @@ -692,7 +696,7 @@ renumber_pivot_task(Context) ->
|| {Id,CatNr} <- Ids
],
ok
end, Context),
end, Context),
{delay, 1}
end.

Expand Down Expand Up @@ -749,3 +753,16 @@ boundaries(CatId, Context) ->
end
end,
z_depcache:memo(F, {category_bounds, CatId}, ?WEEK, [CatId, category], Context).


%% @doc Whether the category tree is currently marked dirty (e.g. resource pivot numbers are being updated)
is_tree_dirty(Context) ->
case m_config:get(?MODULE, meta, Context) of
undefined -> false;
Props ->
proplists:get_value(tree_dirty, Props, false)
end.

%% @doc Set the tree dirty flag
set_tree_dirty(Flag, Context) when Flag =:= true; Flag =:= false ->
m_config:set_prop(?MODULE, meta, tree_dirty, Flag, Context).
109 changes: 74 additions & 35 deletions src/support/z_search.erl
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ search({SearchName, Props} = Search, OffsetLimit, Context) ->
search(Name, OffsetLimit, Context) ->
search({z_convert:to_atom(Name), []}, OffsetLimit, Context).

%% @doc Given a query as proplist, return all results.
%% @spec query_(Props, Context) -> [Id] | []
query_(Props, Context) ->
S = search({'query', Props}, ?OFFSET_PAGING, Context),
S#search_result.result.

%% @doc Handle a return value from a search function. This can be an intermediate SQL statement that still needs to be
%% augmented with extra ACL checks.
Expand Down Expand Up @@ -167,29 +172,29 @@ concat_sql_query(#search_sql{select=Select, from=From, where=Where, group_by=Gro

%% @doc Inject the ACL checks in the SQL query.
%% @spec reformat_sql_query(#search_sql{}, Context) -> #search_sql{}
reformat_sql_query(#search_sql{where=Where, tables=Tables, args=Args, cats=TabCats, cats_exclude=TabCatsExclude} = Q, Context) ->
reformat_sql_query(#search_sql{where=Where, from=From, tables=Tables, args=Args, cats=TabCats, cats_exclude=TabCatsExclude} = Q, Context) ->
{ExtraWhere, Args1} = lists:foldl(
fun(Table, {Acc,As}) ->
{W,As1} = add_acl_check(Table, As, Q, Context),
{[W|Acc], As1}
end, {[], Args}, Tables),
ExtraWhere1 = lists:foldl(
fun({Alias, Cats}, Acc) ->
case add_cat_check(Alias, false, Cats, Context) of
[] -> Acc;
CatCheck -> [CatCheck | Acc]
end
end, ExtraWhere, TabCats),
ExtraWhere2 = lists:foldl(
fun({Alias, Cats}, Acc) ->
case add_cat_check(Alias, true, Cats, Context) of
[] -> Acc;
CatCheck -> [CatCheck | Acc]
{From1, ExtraWhere1} = lists:foldl(
fun({Alias, Cats}, {F, C}) ->
case add_cat_check(F, Alias, false, Cats, Context) of
{_, []} -> {F, C};
{FromNew, CatCheck} -> {FromNew, [CatCheck | C]}
end
end, {From, ExtraWhere}, TabCats),
{From2, ExtraWhere2} = lists:foldl(
fun({Alias, Cats}, {F, C}) ->
case add_cat_check(F, Alias, true, Cats, Context) of
{_, []} -> {F, C};
{FromNew, CatCheck} -> {FromNew, [CatCheck | C]}
end
end, ExtraWhere1, TabCatsExclude),
end, {From1, ExtraWhere1}, TabCatsExclude),

Where1 = lists:flatten(concat_where(ExtraWhere2, Where)),
Q#search_sql{where=Where1, args=Args1}.
Q#search_sql{where=Where1, from=From2, args=Args1}.


%% @doc Concatenate the where clause with the extra ACL checks using "and". Skip empty clauses.
Expand Down Expand Up @@ -288,32 +293,66 @@ publish_check(_Table, _Alias, _SearchSql) ->

%% @doc Create the 'where' conditions for the category check
%% @spec add_cat_check(Alias, Exclude, Cats, Context) -> Where
add_cat_check(_Alias, _Exclude, [], _Context) ->
[];
add_cat_check(Alias, Exclude, Cats, Context) ->
add_cat_check(_From, _Alias, _Exclude, [], _Context) ->
{_From, []};
add_cat_check(From, Alias, Exclude, Cats, Context) ->
case m_category:is_tree_dirty(Context) of
false ->
add_cat_check_pivot(From, Alias, Exclude, Cats, Context);
true ->
lager:warning("dirty: ~p", [dirty]),
%% While the category tree is rebuilding, we use the less efficient version with joins.
add_cat_check_joined(From, Alias, Exclude, Cats, Context)
end.

add_cat_check_pivot(From, Alias, Exclude, Cats, Context) ->
Ranges = m_category:ranges(Cats, Context),
CatChecks = [ cat_check1(Alias, Exclude, Range) || Range <- Ranges ],
CatChecks = [ cat_check_pivot1(Alias, Exclude, Range) || Range <- Ranges ],
case CatChecks of
[] -> [];
[_CatCheck] -> CatChecks;
_ -> "(" ++ string:join(CatChecks, case Exclude of false -> " or "; true -> " and " end) ++ ")"
[] -> {From, []};
[_CatCheck] -> {From, CatChecks};
_ -> {From, "(" ++ string:join(CatChecks, case Exclude of false -> " or "; true -> " and " end) ++ ")"}
end.

cat_check1(Alias, false, {From,From}) ->
Alias ++ ".pivot_category_nr = "++integer_to_list(From);
cat_check1(Alias, false, {From,To}) ->
Alias ++ ".pivot_category_nr >= " ++ integer_to_list(From)
cat_check_pivot1(Alias, false, {From,From}) ->
Alias ++ ".pivot_category_nr = "++integer_to_list(From);
cat_check_pivot1(Alias, false, {From,To}) ->
Alias ++ ".pivot_category_nr >= " ++ integer_to_list(From)
++ " and "++ Alias ++ ".pivot_category_nr <= " ++ integer_to_list(To);

cat_check1(Alias, true, {From,From}) ->
Alias ++ ".pivot_category_nr <> "++integer_to_list(From);
cat_check1(Alias, true, {From,To}) ->
[$(|Alias] ++ ".pivot_category_nr < " ++ integer_to_list(From)
cat_check_pivot1(Alias, true, {From,From}) ->
Alias ++ ".pivot_category_nr <> "++integer_to_list(From);
cat_check_pivot1(Alias, true, {From,To}) ->
[$(|Alias] ++ ".pivot_category_nr < " ++ integer_to_list(From)
++ " or "++ Alias ++ ".pivot_category_nr > " ++ integer_to_list(To) ++ ")".


%% @doc Given a query as proplist, return all results.
%% @spec query_(Props, Context) -> [Id] | []
query_(Props, Context) ->
S = search({'query', Props}, ?OFFSET_PAGING, Context),
S#search_result.result.

%% Add category tree range checks by using joins. Less optimal; only
%% used while the category tree is being recalculated.
add_cat_check_joined(From, Alias, Exclude, Cats, Context) ->
Ranges = m_category:ranges(Cats, Context),
CatAlias = Alias ++ "_cat",
FromNew = [{"category", CatAlias}|From],
CatChecks = lists:map(fun(Range) ->
Check = cat_check_joined1(CatAlias, Exclude, Range),
Alias ++ ".category_id = " ++ CatAlias ++ ".id and " ++ Check
end,
Ranges),
case CatChecks of
[] -> {From, []};
[_CatCheck] -> {FromNew, CatChecks};
_ -> {FromNew, "(" ++ string:join(CatChecks, case Exclude of false -> " or "; true -> " and " end) ++ ")"}
end.

cat_check_joined1(CatAlias, false, {Left,Left}) ->
CatAlias ++ ".nr = "++integer_to_list(Left);
cat_check_joined1(CatAlias, false, {Left,Right}) ->
CatAlias ++ ".nr >= " ++ integer_to_list(Left)
++ " and "++ CatAlias ++ ".nr <= " ++ integer_to_list(Right);

cat_check_joined1(CatAlias, true, {Left,Left}) ->
CatAlias ++ ".nr <> "++integer_to_list(Left);
cat_check_joined1(CatAlias, true, {Left,Right}) ->
"(" ++ CatAlias ++ ".nr < " ++ integer_to_list(Left)
++ " or "++ CatAlias ++ ".nr > " ++ integer_to_list(Right) ++ ")".

0 comments on commit eb0ae0a

Please sign in to comment.