You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by ii...@apache.org on 2020/10/26 10:27:01 UTC

[couchdb] branch main updated: Fix semantics of total_rows

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

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


The following commit(s) were added to refs/heads/main by this push:
     new c0efba3  Fix semantics of total_rows
     new cc9e261  Merge pull request #3227 from cloudant/fix-total_rows-semantics
c0efba3 is described below

commit c0efba32fa1ced66b5baf63ef1ab0dc6188974c5
Author: ILYA Khlopotov <ii...@apache.org>
AuthorDate: Thu Oct 22 08:58:47 2020 -0700

    Fix semantics of total_rows
    
    The `total_rows` field suppose to be a number of documents in the database/view.
    See https://docs.couchdb.org/en/stable/api/ddoc/views.html.
    
    When new pagination API was introduced the meaning of `total_rows` field was
    changed to number of rows in the query results. The PR reverts this
    accidental change.
---
 src/chttpd/test/exunit/pagination_test.exs | 56 +++++++++---------------------
 src/couch_views/src/couch_views_http.erl   | 17 ++++-----
 2 files changed, 23 insertions(+), 50 deletions(-)

diff --git a/src/chttpd/test/exunit/pagination_test.exs b/src/chttpd/test/exunit/pagination_test.exs
index 6544017..4e0a5d6 100644
--- a/src/chttpd/test/exunit/pagination_test.exs
+++ b/src/chttpd/test/exunit/pagination_test.exs
@@ -255,7 +255,7 @@ defmodule Couch.Test.Pagination do
       @describetag descending: descending
       setup [:with_session, :random_db, :with_docs]
 
-      test "total_rows matches the length of rows array", ctx do
+      test "total_rows matches the number of docs", ctx do
         resp =
           Couch.Session.get(ctx.session, "/#{ctx.db_name}/_all_docs",
             query: %{descending: ctx.descending}
@@ -263,7 +263,7 @@ defmodule Couch.Test.Pagination do
 
         assert resp.status_code == 200, "got error #{inspect(resp.body)}"
         body = resp.body
-        assert body["total_rows"] == length(body["rows"])
+        assert body["total_rows"] == ctx.n_docs
       end
 
       test "the rows are correctly sorted", ctx do
@@ -371,7 +371,7 @@ defmodule Couch.Test.Pagination do
     @describetag page_size: 4
     setup [:with_session, :random_db, :with_view, :with_docs]
 
-    test "total_rows matches the length of rows array", ctx do
+    test "total_rows matches the number of documents in view", ctx do
       resp =
         Couch.Session.get(
           ctx.session,
@@ -381,7 +381,7 @@ defmodule Couch.Test.Pagination do
 
       assert resp.status_code == 200, "got error #{inspect(resp.body)}"
       body = resp.body
-      assert body["total_rows"] == length(body["rows"])
+      assert body["total_rows"] == ctx.n_docs
     end
   end
 
@@ -593,14 +593,9 @@ defmodule Couch.Test.Pagination do
           assert Map.has_key?(body, "next")
         end
 
-        test "total_rows matches the length of rows array", ctx do
+        test "total_rows matches the number of documents", ctx do
           body = ctx.response
-          assert body["total_rows"] == length(body["rows"])
-        end
-
-        test "total_rows matches the requested page_size", ctx do
-          body = ctx.response
-          assert body["total_rows"] == ctx.page_size
+          assert body["total_rows"] == ctx.n_docs
         end
 
         test "can use 'next' bookmark to get remaining results", ctx do
@@ -613,8 +608,8 @@ defmodule Couch.Test.Pagination do
 
           assert resp.status_code == 200, "got error #{inspect(resp.body)}"
           body = resp.body
-          assert body["total_rows"] == length(body["rows"])
-          assert body["total_rows"] <= ctx.page_size
+          assert body["total_rows"] == ctx.n_docs
+          assert length(body["rows"]) <= ctx.page_size
         end
       end
 
@@ -721,7 +716,7 @@ defmodule Couch.Test.Pagination do
 
         test "final page doesn't include 'next' bookmark", ctx do
           assert not Map.has_key?(ctx.response, "next")
-          assert ctx.response["total_rows"] == rem(ctx.n_docs, ctx.page_size)
+          assert length(ctx.response["rows"]) == rem(ctx.n_docs, ctx.page_size)
         end
 
         test "each but last page has page_size rows", ctx do
@@ -768,14 +763,9 @@ defmodule Couch.Test.Pagination do
         assert not Map.has_key?(body, "next")
       end
 
-      test "total_rows matches the length of rows array", ctx do
-        body = ctx.response
-        assert body["total_rows"] == length(body["rows"])
-      end
-
-      test "total_rows less than the requested page_size", ctx do
+      test "total_rows matches the number of documents", ctx do
         body = ctx.response
-        assert body["total_rows"] <= ctx.page_size
+        assert body["total_rows"] == ctx.n_docs
       end
     end
   end
@@ -950,7 +940,7 @@ defmodule Couch.Test.Pagination do
           assert not Map.has_key?(resp.body, "previous")
         end
 
-        test "total_rows matches the length of rows array", ctx do
+        test "total_rows matches the number of documents in view", ctx do
           resp =
             Couch.Session.get(
               ctx.session,
@@ -960,19 +950,7 @@ defmodule Couch.Test.Pagination do
 
           assert resp.status_code == 200, "got error #{inspect(resp.body)}"
           body = resp.body
-          assert body["total_rows"] == length(body["rows"])
-        end
-
-        test "total_rows matches the requested page_size", ctx do
-          resp =
-            Couch.Session.get(
-              ctx.session,
-              "/#{ctx.db_name}/_design/#{ctx.ddoc_id}/_view/#{ctx.view_name}",
-              query: %{page_size: ctx.page_size, descending: ctx.descending}
-            )
-
-          assert resp.status_code == 200, "got error #{inspect(resp.body)}"
-          assert resp.body["total_rows"] == ctx.page_size
+          assert body["total_rows"] == ctx.n_docs
         end
 
         test "can use 'next' bookmark to get remaining results", ctx do
@@ -994,8 +972,8 @@ defmodule Couch.Test.Pagination do
 
           assert resp.status_code == 200, "got error #{inspect(resp.body)}"
           body = resp.body
-          assert body["total_rows"] == length(body["rows"])
-          assert body["total_rows"] <= ctx.page_size
+          assert body["total_rows"] == ctx.n_docs
+          assert length(body["rows"]) <= ctx.page_size
         end
 
         test "can use 'previous' bookmark", ctx do
@@ -1065,7 +1043,7 @@ defmodule Couch.Test.Pagination do
 
         assert resp.status_code == 200, "got error #{inspect(resp.body)}"
         body = resp.body
-        assert body["total_rows"] == length(body["rows"])
+        assert body["total_rows"] == ctx.n_docs
       end
 
       test "total_rows less than the requested page_size", ctx do
@@ -1077,7 +1055,7 @@ defmodule Couch.Test.Pagination do
           )
 
         assert resp.status_code == 200, "got error #{inspect(resp.body)}"
-        assert resp.body["total_rows"] <= ctx.page_size
+        assert length(resp.body["rows"]) <= ctx.page_size
       end
     end
   end
diff --git a/src/couch_views/src/couch_views_http.erl b/src/couch_views/src/couch_views_http.erl
index e21acfb..769d8c3 100644
--- a/src/couch_views/src/couch_views_http.erl
+++ b/src/couch_views/src/couch_views_http.erl
@@ -95,9 +95,8 @@ paginated_cb({meta, Meta}, #vacc{}=VAcc) ->
         case MetaData of
             {_Key, undefined} ->
                 Acc;
-            {total, _Value} ->
-                %% We set total_rows elsewere
-                Acc;
+            {total, Value} ->
+                maps:put(total_rows, Value, Acc);
             {Key, Value} ->
                 maps:put(list_to_binary(atom_to_list(Key)), Value, Acc)
         end
@@ -129,14 +128,12 @@ do_paginated(PageSize, QueriesArgs, KeyFun, Fun) when is_list(QueriesArgs) ->
                 Result0 = maybe_add_next_bookmark(
                     OriginalLimit, PageSize, Args, Meta, Items, KeyFun),
                 Result = maybe_add_previous_bookmark(Args, Result0, KeyFun),
-                #{total_rows := Total} = Result,
-                {Limit - Total, [Result | Acc]};
+                {Limit - length(maps:get(rows, Result)), [Result | Acc]};
             false ->
                 Bookmark = bookmark_encode(Args0),
                 Result = #{
                     rows => [],
-                    next => Bookmark,
-                    total_rows => 0
+                    next => Bookmark
                 },
                 {Limit, [Result | Acc]}
         end
@@ -152,8 +149,7 @@ maybe_add_next_bookmark(OriginalLimit, PageSize, Args0, Response, Items, KeyFun)
     case check_completion(OriginalLimit, RequestedLimit, Items) of
         {Rows, nil} ->
             maps:merge(Response, #{
-                rows => Rows,
-                total_rows => length(Rows)
+                rows => Rows
             });
         {Rows, Next} ->
             {FirstId, FirstKey} = first_key(KeyFun, Rows),
@@ -169,8 +165,7 @@ maybe_add_next_bookmark(OriginalLimit, PageSize, Args0, Response, Items, KeyFun)
             Bookmark = bookmark_encode(Args),
             maps:merge(Response, #{
                 rows => Rows,
-                next => Bookmark,
-                total_rows => length(Rows)
+                next => Bookmark
             })
     end.