apache / couchdb

Seamless multi-master syncing database with an intuitive HTTP/JSON API, designed for reliability
https://couchdb.apache.org/
Apache License 2.0
6.22k stars 1.03k forks source link

Nouveau range faceting doesn’t work as documented #5273

Closed espy closed 2 weeks ago

espy commented 2 weeks ago

Description

The range faceting syntax in the Nouveau docs doesn’t work. It describes the same ranges syntax as the ddocs search page, but this doesn’t actually work with Nouveau.

I’m not sure whether the Nouveau docs are wrong, or Nouveau is missing the intended implementation. There is a working syntax described elsewhere.

In addition, this other working syntax has confusing defaults for inclusive/exclusive range facets (described further below).

Steps to Reproduce

In a CouchDB with activated Nouveau, with a running Nouveau server:

  1. Have some docs with a number value, eg. rating: 8.1
  2. Add a Nouveau double index for that key:
    {
      "_id": "_design/nouveau_example",
      "nouveau": {
        "range": {
          "index": "if (doc.rating) { index('double', 'rating', doc.rating) } }"
        }
      }
    }
  3. Send a ranges query as described in the Nouveau Docs, but without using Infinity, because that also doesn’t work as documented, see this issue:

    Taken from the docs:

    ?q=*:*&ranges={"price":{"cheap":"[0 TO 100]","expensive":"{100 TO Infinity}"}}

    …adapted to use our data and 10 instead of Infinity:

    ?q=*:*&ranges={"rating":{"boring":"[0 TO 8]","amazing":"{8 TO 10}"}}

    As JSON, for readability:

    {
      "rating": {
        "boring": "[0 TO 8]",
        "amazing": "{8 TO 10}"
      }
    }
  4. This returns:
    {
      "error": "query_parse_error",
      "reason": "range values must be lists of objects"
    }

Turning the query into a list of objects doesn’t work though. What does work is the syntax found in this Nouveau README.md:

?q=*:*&ranges={ "rating": [ { "label": "boring", "min": 0, "max": 8 }, { "label": "amazing", "min": 8, "max": 10 } ] }

as JSON, for readability:

{
  "rating": [
    {
      "label": "boring",
      "min": 0,
      "max": 8
    },
    {
      "label": "amazing",
      "min": 8,
      "max": 10
    }
  ]
}

This returns a result:

 "total_hits": 23,
  "ranges": {
    "rating": {
      "boring": 16,
      "amazing": 9
    }
  },

Now, you’ll notice that the total count in the facets is more than the total_hits (25 vs 23), this is because the rating 8 falls into both facets, so all hits with a rating of 8 are counted twice. The Nouveau implementation of ranges can also use min_inclusive and max_inclusive boolean keys, which (to me) implies that min and max are by default exclusive, however we just saw they aren’t: 8 is included in both min and max, and the default for both booleans is actually true.

Using min_inclusive and max_inclusive does then work. Assigning the 8 to the amazing facet like so:

?q=*:*&ranges={ "rating": [ { "label": "boring", "min": 0, "max": 8, "max_inclusive": false }, { "label": "amazing", "min": 8, "max": 10 } ] }

returns:

  "total_hits": 23,
  "ranges": {
    "rating": {
      "boring": 14,
      "amazing": 9
    }
  },

Expected Behaviour

I expected the example in the docs to work.

Your Environment

{
  "couchdb":"Welcome",
  "version":"3.4.1",
  "git_sha":"f504e38a5",
  "uuid":"4e36c505782b7068ead9895902c5f07d",
  "features":[
    "nouveau",
    "access-ready",
    "partitioned",
    "pluggable-storage-engines",
    "reshard",
    "scheduler"
  ],
  "vendor":{"name":"The Apache Software Foundation"}
}
espy commented 2 weeks ago

Just found this Nouveau API docs page which has the correct syntax, so I suppose the docs on the main Nouveau page are simply out of date.

rnewson commented 2 weeks ago

I was just getting to this ticket. Yes, the nouveau syntax here is different. I will confirm it all works as documented now.

rnewson commented 2 weeks ago

Thank you for the feedback btw, this is exactly the sort of thing I was hoping for in the first release. Hopefully it's all just little documentation oopsies...