Add key range check for _all_docs

[jiahuili430]

Overview

Using key and start/end_key in a different order will produce different responses when querying _all_docs.
To reduce confusion, add a key range check for _all_docs.

• If direction=fwd, start_key > end_key throws an error
• If direction=rev, start_key < end_key throws an error
• Otherwise, return relevant responses

Previous behavior:

$ curl -XPUT $DB/db
$ curl -XPOST $DB/db/_bulk_docs -H 'Content-Type: application/json' -d '{"docs": [{"_id": "a"},{"_id": "b"},{"_id": "c"}]}'

$ curl $DB/db/_all_docs'?key="c"&endkey="a"'
{"total_rows":3,"offset":2,"rows":[ ]}

$ curl $DB/db/_all_docs'?key="c"&endkey="a"&descending=true'
{"total_rows":3,"offset":0,"rows":[
  {"id":"c","key":"c","value":{"rev":"1-967a00dff5e02add41819138abb3284d"}},
  {"id":"b","key":"b","value":{"rev":"1-967a00dff5e02add41819138abb3284d"}},
  {"id":"a","key":"a","value":{"rev":"1-967a00dff5e02add41819138abb3284d"}}
]}

After the change:

$ curl $DB/db/_all_docs'?key="c"&endkey="a"'
{"error":"query_parse_error","reason":"No rows can match your key range, reverse your start_key and end_key or set descending=true"}

$ curl $DB/db/_all_docs'?key="c"&endkey="a"&descending=true'
{"total_rows":3,"offset":0,"rows":[
  {"id":"c","key":"c","value":{"rev":"1-967a00dff5e02add41819138abb3284d"}},
  {"id":"b","key":"b","value":{"rev":"1-967a00dff5e02add41819138abb3284d"}},
  {"id":"a","key":"a","value":{"rev":"1-967a00dff5e02add41819138abb3284d"}}
]}

Testing recommendations

make eunit apps=chttpd,couch_mrview

Related Issues or Pull Requests

#3977

Checklist

• Code is written and works correctly
• Changes are covered by tests
• Any new configurable parameters are documented in rel/overlay/etc/default.ini
• Documentation changes were made in the src/docs folder
• Documentation changes were backported (separated PR) to affected branches

[rnewson]

the change itself looks fine (though I have a question on why this extra argument validation is not happening inside validate_args/1) but the backward compatibility issue needs discussing. This change means a user will now get an error when previously they got success (with no rows).

[rnewson]

why not put this inside validate_args?

[jiahuili430]

Tried that before but it fails in mango tests related to _find endpoint.
https://ci-couchdb.apache.org/blue/organizations/jenkins/jenkins-cm1%2FPullRequests/detail/PR-4518/9/pipeline/

ERROR: test_gte_respsects_unicode_collation (03-operator-test.OperatorJSONTests)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/home/jenkins/workspace/jenkins-cm1_PullRequests_PR-4518/25.3/build/apache-couchdb-3.3.2-04ead65/src/mango/test/03-operator-test.py", line 187, in test_gte_respsects_unicode_collation
   docs = self.db.find({"ordered": {"$gte": "a"}})
 File "/home/jenkins/workspace/jenkins-cm1_PullRequests_PR-4518/25.3/build/apache-couchdb-3.3.2-04ead65/src/mango/test/mango.py", line 280, in find
   r.raise_for_status()
 File "/home/jenkins/workspace/jenkins-cm1_PullRequests_PR-4518/25.3/build/apache-couchdb-3.3.2-04ead65/src/mango/.venv/lib/python3.9/site-packages/requests/models.py", line 960, in raise_for_status
   raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: http://127.0.0.1:15984/mango_test_cad95f52e95341e097ce2c1d861c1457/_find
======================================================================
ERROR: test_old_selector_with_no_selector_still_supported (16-index-selectors-test.IndexSelectorJson)
... ...

[janl]

This change means a user will now get an error when previously they got success (with no rows).

With more of these cleanups coming in (thanks @jiahuili430!) we may have to bite the bullet and think about API versioning. We have discussed several approaches in the past but nothing has ever stuck.

It might be worth bringing this up in the next developer meeting for a high-level discussion. Until we have a better idea, the best idea I’d have is to put this (and other improvements) behind a [chttpd] strict_api = false | true or some such config flag, but it might be better to hold this PR until we have a general solution for this.

[rnewson]

I like the idea of a flag but wouldn't we need a roadmap for the full set of changes first? (in a python 2/3 way)

[janl]

the flag would be a stopgap until we have proper versioning, it’d be a toggle between version 0 and 1, and later we allow versions 2, 3, 4 etc. once we know what these look like. — I wouldn’t mind parking this PR until we have a proper system set up and skip the binary flag for now, but if we would want the PR to land quickly, that’d be the fasted way there, but yeah, we probably want a complete scrub before we introduce even the toggle.