You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2024/10/11 01:48:52 UTC
(couchdb) 04/10: nouveau: Add ?top_n=X query parameter for facets
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch catch-up-3.4.x
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 6bd77e2e87fcdf0651b487f18185ea1e539858b6
Author: Robert Newson <rn...@apache.org>
AuthorDate: Tue Oct 8 14:09:15 2024 +0100
nouveau: Add ?top_n=X query parameter for facets
---
.../apache/couchdb/nouveau/api/SearchRequest.java | 2 +-
.../couchdb/nouveau/api/SearchRequestTest.java | 1 +
.../couchdb/nouveau/lucene9/Lucene9IndexTest.java | 29 ++++++++++++
.../src/test/resources/fixtures/SearchRequest.json | 3 +-
src/docs/src/api/ddoc/nouveau.rst | 5 +-
src/docs/src/ddocs/nouveau.rst | 3 ++
src/nouveau/src/nouveau_fabric_search.erl | 53 +++++++++++++++++++---
src/nouveau/src/nouveau_httpd.erl | 16 +++++++
src/nouveau/src/nouveau_int.hrl | 16 +++++++
test/elixir/test/config/nouveau.elixir | 1 +
test/elixir/test/nouveau_test.exs | 17 +++++++
11 files changed, 135 insertions(+), 11 deletions(-)
diff --git a/nouveau/src/main/java/org/apache/couchdb/nouveau/api/SearchRequest.java b/nouveau/src/main/java/org/apache/couchdb/nouveau/api/SearchRequest.java
index 155e8efb4..8e4ebbf8d 100644
--- a/nouveau/src/main/java/org/apache/couchdb/nouveau/api/SearchRequest.java
+++ b/nouveau/src/main/java/org/apache/couchdb/nouveau/api/SearchRequest.java
@@ -55,7 +55,7 @@ public class SearchRequest {
private PrimitiveWrapper<?>[] after;
@Min(1)
- @Max(100)
+ @Max(1000)
private int topN = 10;
public SearchRequest() {
diff --git a/nouveau/src/test/java/org/apache/couchdb/nouveau/api/SearchRequestTest.java b/nouveau/src/test/java/org/apache/couchdb/nouveau/api/SearchRequestTest.java
index 01489b021..9269f11e8 100644
--- a/nouveau/src/test/java/org/apache/couchdb/nouveau/api/SearchRequestTest.java
+++ b/nouveau/src/test/java/org/apache/couchdb/nouveau/api/SearchRequestTest.java
@@ -51,6 +51,7 @@ public class SearchRequestTest {
result.setQuery("*:*");
result.setLimit(10);
result.setCounts(List.of("bar"));
+ result.setTopN(5);
result.setRanges(Map.of("foo", List.of(new DoubleRange("0 to 100 inc", 0.0, true, 100.0, true))));
return result;
}
diff --git a/nouveau/src/test/java/org/apache/couchdb/nouveau/lucene9/Lucene9IndexTest.java b/nouveau/src/test/java/org/apache/couchdb/nouveau/lucene9/Lucene9IndexTest.java
index c3c7744b4..428e3eb6a 100644
--- a/nouveau/src/test/java/org/apache/couchdb/nouveau/lucene9/Lucene9IndexTest.java
+++ b/nouveau/src/test/java/org/apache/couchdb/nouveau/lucene9/Lucene9IndexTest.java
@@ -157,6 +157,35 @@ public class Lucene9IndexTest {
}
}
+ @Test
+ public void testTopN(@TempDir Path path) throws IOException {
+ Index index = setup(path);
+ try {
+ final int count = 50;
+ for (int i = 1; i <= count; i++) {
+ final Collection<Field> fields = List.of(new StringField("bar", "bar", false));
+ final DocumentUpdateRequest request = new DocumentUpdateRequest(i - 1, i, null, fields);
+ index.update("doc" + i, request);
+ }
+
+ for (int i = count + 1; i <= (count * 2) + 5; i++) {
+ final Collection<Field> fields = List.of(new StringField("bar", "baz", false));
+ final DocumentUpdateRequest request = new DocumentUpdateRequest(i - 1, i, null, fields);
+ index.update("doc" + i, request);
+ }
+
+ final SearchRequest request = new SearchRequest();
+ request.setMinUpdateSeq(count);
+ request.setQuery("*:*");
+ request.setCounts(List.of("bar"));
+ request.setTopN(1);
+ final SearchResults results = index.search(request);
+ assertThat(results.getCounts()).isEqualTo(Map.of("bar", Map.of("baz", count + 5)));
+ } finally {
+ cleanup(index);
+ }
+ }
+
@Test
public void testOutOfOrder(@TempDir Path path) throws IOException {
Index index = setup(path);
diff --git a/nouveau/src/test/resources/fixtures/SearchRequest.json b/nouveau/src/test/resources/fixtures/SearchRequest.json
index c588cc16b..2583fb2ad 100644
--- a/nouveau/src/test/resources/fixtures/SearchRequest.json
+++ b/nouveau/src/test/resources/fixtures/SearchRequest.json
@@ -5,6 +5,7 @@
"counts": [
"bar"
],
+ "top_n": 5,
"ranges": {
"foo": [
{
@@ -14,4 +15,4 @@
}
]
}
-}
\ No newline at end of file
+}
diff --git a/src/docs/src/api/ddoc/nouveau.rst b/src/docs/src/api/ddoc/nouveau.rst
index f80229b60..286b9a0d3 100644
--- a/src/docs/src/api/ddoc/nouveau.rst
+++ b/src/docs/src/api/ddoc/nouveau.rst
@@ -50,8 +50,7 @@
Defaults to the JDK default locale if not specified. Some examples are ``de``
, ``us``, ``gb``.
:query number limit: Limit the number of the returned documents to the specified
- number. For a grouped search, this parameter limits the number of documents per
- group.
+ number.
:query string q: Required. The Lucene query string.
:query json ranges: This field defines ranges for numeric search fields. The
value is a JSON object where the fields names are numeric search fields,
@@ -68,6 +67,8 @@
same order as the array.
Some examples are ``"relevance"``, ``"bar"``, ``"-foo"`` and
[``"-foo"``, ``"bar"``].
+ :query number top_n: Limit the number of facets returned by group, defaulting to 10
+ with a maximum of 1000.
:query boolean update: Set to ``false`` to allow the use of an out-of-date index.
:>header Content-Type: - :mimetype:`application/json`
diff --git a/src/docs/src/ddocs/nouveau.rst b/src/docs/src/ddocs/nouveau.rst
index 4f21c1455..949cd17a2 100644
--- a/src/docs/src/ddocs/nouveau.rst
+++ b/src/docs/src/ddocs/nouveau.rst
@@ -636,6 +636,9 @@ field. If you do not create separate indexes for each field, you must include on
documents that contain all the fields. Verify that the fields exist in each document by
using a single ``if`` statement.
+The ``top_n`` query parameter controls how many facets, per grouping, are returned,
+defaulting to 10, to a maximum of 1000.
+
*Example if statement to verify that the required fields exist in each document:*
.. code-block:: javascript
diff --git a/src/nouveau/src/nouveau_fabric_search.erl b/src/nouveau/src/nouveau_fabric_search.erl
index 4ced63e14..fdc2d42fc 100644
--- a/src/nouveau/src/nouveau_fabric_search.erl
+++ b/src/nouveau/src/nouveau_fabric_search.erl
@@ -19,6 +19,7 @@
-include_lib("mem3/include/mem3.hrl").
-include_lib("couch/include/couch_db.hrl").
+-include("nouveau_int.hrl").
-record(state, {
limit,
@@ -39,13 +40,20 @@ go(DbName, #doc{} = DDoc, IndexName, QueryArgs0) ->
Shards = get_shards(DbName, QueryArgs0),
{PackedBookmark, #{limit := Limit, sort := Sort} = QueryArgs1} =
maps:take(bookmark, QueryArgs0),
+ TopN = maps:get(top_n, QueryArgs1, ?TOP_N_DEFAULT),
Bookmark = nouveau_bookmark:unpack(DbName, PackedBookmark),
Counters0 = lists:map(
fun(#shard{} = Shard) ->
After = maps:get(Shard#shard.range, Bookmark, null),
+ %% scale TopN by number of shards so we don't clip a global topN
+ %% label just because it wasn't in the topN locally.
Ref = rexi:cast(
Shard#shard.node,
- {nouveau_rpc, search, [Shard#shard.name, Index, QueryArgs1#{'after' => After}]}
+ {nouveau_rpc, search, [
+ Shard#shard.name,
+ Index,
+ QueryArgs1#{'after' => After, top_n => TopN * length(Shards)}
+ ]}
),
Shard#shard{ref = Ref}
end,
@@ -72,7 +80,7 @@ go(DbName, #doc{} = DDoc, IndexName, QueryArgs0) ->
of
{ok, SearchResults} ->
NewBookmark = nouveau_bookmark:update(DbName, Bookmark, SearchResults),
- {ok, simplify_hits(SearchResults#{bookmark => NewBookmark})};
+ {ok, top_n_facets(simplify_hits(SearchResults#{bookmark => NewBookmark}), TopN)};
{error, Reason} ->
{error, Reason}
after
@@ -126,10 +134,10 @@ merge_search_results(A, B, #state{} = State) ->
State#state.limit
),
<<"counts">> => merge_facets(
- maps:get(<<"counts">>, A, null), maps:get(<<"counts">>, B, null), State#state.limit
+ maps:get(<<"counts">>, A, null), maps:get(<<"counts">>, B, null)
),
<<"ranges">> => merge_facets(
- maps:get(<<"ranges">>, A, null), maps:get(<<"ranges">>, B, null), State#state.limit
+ maps:get(<<"ranges">>, A, null), maps:get(<<"ranges">>, B, null)
)
}.
@@ -215,11 +223,42 @@ convert_item(Item) ->
maps:get(<<"value">>, Item)
end.
-merge_facets(FacetsA, null, _Limit) ->
+top_n_facets(SearchResults, TopN) ->
+ SearchResults#{
+ <<"counts">> => top_n_facets(<<"counts">>, SearchResults, TopN),
+ <<"ranges">> => top_n_facets(<<"ranges">>, SearchResults, TopN)
+ }.
+
+top_n_facets(Key, SearchResults, TopN) ->
+ Facets = maps:get(Key, SearchResults, null),
+ top_n_facets_int(Facets, TopN).
+
+top_n_facets_int(null, _TopN) ->
+ null;
+top_n_facets_int(Facets, TopN) when is_map(Facets), is_integer(TopN), TopN > 0 ->
+ MapFun = fun(_, Group) ->
+ maps:from_list(
+ lists:sublist(
+ lists:sort(fun facet_sort/2, maps:to_list(Group)),
+ TopN
+ )
+ )
+ end,
+ maps:map(MapFun, Facets).
+
+%% sort by value (high to low), tie-break on label (lowest wins)
+facet_sort({_K1, V1}, {_K2, V2}) when V1 > V2 ->
+ true;
+facet_sort({_K1, V1}, {_K2, V2}) when V1 < V2 ->
+ false;
+facet_sort({K1, V}, {K2, V}) ->
+ couch_ejson_compare:less_json(K1, K2).
+
+merge_facets(FacetsA, null) ->
FacetsA;
-merge_facets(null, FacetsB, _Limit) ->
+merge_facets(null, FacetsB) ->
FacetsB;
-merge_facets(FacetsA, FacetsB, _Limit) ->
+merge_facets(FacetsA, FacetsB) ->
Combiner = fun(_, V1, V2) -> maps:merge_with(fun(_, V3, V4) -> V3 + V4 end, V1, V2) end,
maps:merge_with(Combiner, FacetsA, FacetsB).
diff --git a/src/nouveau/src/nouveau_httpd.erl b/src/nouveau/src/nouveau_httpd.erl
index 10917b3d4..0290a620b 100644
--- a/src/nouveau/src/nouveau_httpd.erl
+++ b/src/nouveau/src/nouveau_httpd.erl
@@ -16,6 +16,7 @@
-module(nouveau_httpd).
-include_lib("couch/include/couch_db.hrl").
+-include("nouveau_int.hrl").
-export([
handle_analyze_req/1,
@@ -68,6 +69,7 @@ handle_search_req_int(#httpd{method = 'GET', path_parts = [_, _, _, _, IndexName
locale => chttpd:qs_value(Req, "locale"),
partition => chttpd:qs_value(Req, "partition"),
limit => chttpd:qs_value(Req, "limit"),
+ top_n => chttpd:qs_value(Req, "top_n"),
sort => chttpd:qs_value(Req, "sort"),
ranges => chttpd:qs_value(Req, "ranges"),
counts => chttpd:qs_value(Req, "counts"),
@@ -87,6 +89,7 @@ handle_search_req_int(
locale => maps:get(<<"locale">>, ReqBody, undefined),
partition => chttpd:qs_value(Req, "partition"),
limit => maps:get(<<"limit">>, ReqBody, undefined),
+ top_n => maps:get(<<"top_n">>, ReqBody, undefined),
sort => json_or_undefined(<<"sort">>, ReqBody),
ranges => json_or_undefined(<<"ranges">>, ReqBody),
counts => json_or_undefined(<<"counts">>, ReqBody),
@@ -227,6 +230,19 @@ validate_query_arg(update, undefined) ->
true;
validate_query_arg(update, Bool) when is_boolean(Bool) ->
Bool;
+validate_query_arg(top_n, undefined) ->
+ ?TOP_N_DEFAULT;
+validate_query_arg(top_n, TopN) when is_integer(TopN), TopN > 0 ->
+ TopN;
+validate_query_arg(top_n, TopN) when is_integer(TopN) ->
+ throw({query_parse_error, <<"top_n parameter must be greater than zero">>});
+validate_query_arg(top_n, TopN) when is_list(TopN) ->
+ try
+ list_to_integer(TopN)
+ catch
+ error:badarg ->
+ throw({query_parse_error, <<"top_n parameter must be an integer">>})
+ end;
validate_query_arg(update, "false") ->
false;
validate_query_arg(update, "true") ->
diff --git a/src/nouveau/src/nouveau_int.hrl b/src/nouveau/src/nouveau_int.hrl
new file mode 100644
index 000000000..ea4ba6f29
--- /dev/null
+++ b/src/nouveau/src/nouveau_int.hrl
@@ -0,0 +1,16 @@
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+
+%% -*- erlang-indent-level: 4;indent-tabs-mode: nil -*-
+
+-define(TOP_N_DEFAULT, 10).
diff --git a/test/elixir/test/config/nouveau.elixir b/test/elixir/test/config/nouveau.elixir
index 1ea4605c0..55c514167 100644
--- a/test/elixir/test/config/nouveau.elixir
+++ b/test/elixir/test/config/nouveau.elixir
@@ -16,6 +16,7 @@
"counts",
"ranges",
"ranges (open)",
+ "top_n",
"mango search by number",
"mango search by string",
"mango search by text",
diff --git a/test/elixir/test/nouveau_test.exs b/test/elixir/test/nouveau_test.exs
index 5780fa55c..32162330b 100644
--- a/test/elixir/test/nouveau_test.exs
+++ b/test/elixir/test/nouveau_test.exs
@@ -379,6 +379,23 @@ defmodule NouveauTest do
assert ranges == %{"bar" => %{"cheap" => 3, "expensive" => 1}}
end
+ @tag :with_db
+ test "top_n", context do
+ db_name = context[:db_name]
+ create_search_docs(db_name)
+ create_ddoc(db_name)
+
+ url = "/#{db_name}/_design/foo/_nouveau/bar"
+ resp = Couch.post(url, body: %{q: "*:*", ranges: %{bar: [
+ %{label: "cheap", max: 42},
+ %{label: "expensive", min: 42, min_inclusive: false}]},
+ top_n: 1,
+ include_docs: true})
+ assert_status_code(resp, 200)
+ %{:body => %{"ranges" => ranges}} = resp
+ assert ranges == %{"bar" => %{"cheap" => 3}}
+ end
+
@tag :with_db
test "mango search by number", context do
db_name = context[:db_name]