You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by rn...@apache.org on 2024/10/08 15:05:12 UTC

(couchdb) branch top_n updated (4d4309486 -> e0c64c2f6)

This is an automated email from the ASF dual-hosted git repository.

rnewson pushed a change to branch top_n
in repository https://gitbox.apache.org/repos/asf/couchdb.git


 discard 4d4309486 nouveau: Add ?top_n=X query parameter for facets
     new e0c64c2f6 nouveau: Add ?top_n=X query parameter for facets

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (4d4309486)
            \
             N -- N -- N   refs/heads/top_n (e0c64c2f6)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../couchdb/nouveau/lucene9/Lucene9IndexTest.java  | 29 ++++++++++++++++++++++
 src/nouveau/src/nouveau_fabric_search.erl          | 23 ++++++-----------
 test/elixir/test/config/nouveau.elixir             |  1 +
 test/elixir/test/nouveau_test.exs                  | 17 +++++++++++++
 4 files changed, 55 insertions(+), 15 deletions(-)


(couchdb) 01/01: nouveau: Add ?top_n=X query parameter for facets

Posted by rn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rnewson pushed a commit to branch top_n
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit e0c64c2f6e07a68015867bd1790a6d931533976e
Author: Robert Newson <rn...@apache.org>
AuthorDate: Tue Oct 8 14:09:15 2024 +0100

    nouveau: Add ?top_n=X query parameter for facets
---
 .../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                  |  4 +-
 src/docs/src/ddocs/nouveau.rst                     |  3 ++
 src/nouveau/src/nouveau_fabric_search.erl          | 53 +++++++++++++++++++---
 src/nouveau/src/nouveau_httpd.erl                  | 15 ++++++
 test/elixir/test/config/nouveau.elixir             |  1 +
 test/elixir/test/nouveau_test.exs                  | 17 +++++++
 9 files changed, 116 insertions(+), 10 deletions(-)

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..8a651b8c0 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,7 @@
         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.
     :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..831797877 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.
+
 *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 5008df9d6..8210eb12d 100644
--- a/src/nouveau/src/nouveau_fabric_search.erl
+++ b/src/nouveau/src/nouveau_fabric_search.erl
@@ -39,13 +39,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, 10),
     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 +79,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 +133,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)
         )
     }.
 
@@ -212,11 +219,43 @@ convert_item(Item) ->
             maps:get(<<"value">>, Item)
     end.
 
-merge_facets(FacetsA, null, _Limit) ->
+top_n_facets(SearchResults, TopN) ->
+    After = SearchResults#{
+        <<"counts">> => top_n_facets(<<"counts">>, SearchResults, TopN),
+        <<"ranges">> => top_n_facets(<<"ranges">>, SearchResults, TopN)
+    },
+    After.
+
+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..d17ec9e2a 100644
--- a/src/nouveau/src/nouveau_httpd.erl
+++ b/src/nouveau/src/nouveau_httpd.erl
@@ -68,6 +68,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 +88,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 +229,19 @@ validate_query_arg(update, undefined) ->
     true;
 validate_query_arg(update, Bool) when is_boolean(Bool) ->
     Bool;
+validate_query_arg(top_n, undefined) ->
+    10;
+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/test/elixir/test/config/nouveau.elixir b/test/elixir/test/config/nouveau.elixir
index bff40e84f..e9d6f91f7 100644
--- a/test/elixir/test/config/nouveau.elixir
+++ b/test/elixir/test/config/nouveau.elixir
@@ -15,6 +15,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 0fb280efb..75fef35f3 100644
--- a/test/elixir/test/nouveau_test.exs
+++ b/test/elixir/test/nouveau_test.exs
@@ -348,6 +348,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]