Mango match_failures/2 function - #6080
Open
jcoglan wants to merge 16 commits into
Open
Conversation
jcoglan
force-pushed
the
mango-match-failures
branch
from
July 31, 2026 17:18
5404017 to
200cbda
Compare
nickva
requested changes
Aug 6, 2026
nickva
left a comment
nickva
left a comment
Contributor
There was a problem hiding this comment.
Looks great. Almost ready to merge!
Just a few more comments, a way to generalize the error a bit maybe, and a comment about always validating mango VDUs to remove a footgun from the user.
Also wondering if we can re-run the perf tests with the latest shape since we more clauses to handle.
| throw:{mango_error, mango_selector, {invalid_operator, Op}} -> | ||
| Msg = io_lib:format("invalid operator: ~p", [Op]), | ||
| {reply, {error, {compilation_error, Msg}}, St}; | ||
| throw:{mango_error, mango_util, {invalid_field_name, Field}} -> |
Contributor
There was a problem hiding this comment.
Wonder if this will catch all possible mango_error errors. What about {mango_error, mango_selector, {bad_arg, ...}}. Maybe we can just do
throw:{mango_error, _Mod, Reason} ->
Msg = io_lib:format("invalid selector: ~p", [Reason]),
{reply, {error, {compilation_error, Msg}}, St}
So we don't have to worry about every single corner case here
Contributor
Author
There was a problem hiding this comment.
This results in responses containing literal Erlang values, which I'd rather avoid. I'm not sure how else to write a generic error formatting block that does not have this problem.
$ cdb '/asd/_design/vdu' -X PUT -d '{ "language": "query", "validate_doc_update": { "newDoc.x": { "$nope": 0 } } }'
{"error":"compilation_error","reason":"Compilation of the validate_doc_update function in the 'validate_doc_update' view failed: invalid selector: {invalid_operator,<<\"$nope\">>}"}
Contributor
There was a problem hiding this comment.
This results in responses containing literal Erlang values, which I'd rather avoid. I'm not sure how else to write a generic error formatting block that does not have this problem.
We do have a function to format those in a dedicated module, that might work
{_Code, _Name, Msg} = mango_error:info(Mod, Reason),
{reply, {error, {compilation_error, Msg}}, St}Even if we didn't it might still be preferable to return some ugly error which makes sense rather than a 500 crash which doesn't
| end. | ||
|
|
||
| should_validate_vdu(#doc{body = {Props}}) -> | ||
| case config:get_boolean("couchdb", "validate_vdu", false) of |
Contributor
There was a problem hiding this comment.
As discussed in slack let's always validate mango VDUs to remove a footgun from the users
Contributor
Author
There was a problem hiding this comment.
I've just amended things so that Mango VDUs are always validated.
Rather than returning a boolean to indicate just success or failure, `mango_selector:match/2` now returns a list of "failures" describing the ways in which the selector failed to match the input. If this list is empty, the match was a success.
We will need to pass other things around between `match` calls as well the current `Cmp` function, so here we replace this argument with a `#ctx` record that intially just contains a `cmp` field.
To give detailed feedback to the caller, the `#ctx` argument to `mango_selector:match/3` now records the path that was taken to reach each value, and this path is added to the `#failure` records. Each path segment is either a binary, if it represents an object property, or an integer if it represents an array index. Items are pushed on the front of `#ctx.path` as this is faster than pushing onto the back of a list. This list can then be reversed once the final list of failures has been generated, before the failures are presented to the caller.
Collecting detailed `#failure` records rather than a boolean true/false
when evaluating selectors imposes a performance penalty, so we would
like to only do this when a selector is used for a VDU, not when it is
used for indexing/filtering.
To this end we introduce "verbose" mode signalled via the `#ctx.verbose`
field, and each branch of `mango_selector:match/3` now has 3 distinct
versions:
- `#ctx{verbose = false}`: this is the original version that returns
true/false, taken when a selector is used for Mango queries.
- `#ctx{verbose = true, negate = false}`: verbose mode, when the
operator is not negated by an enclosing `$not` operator. Returns a
list of `#failure` records which may be empty.
- `#ctx{verbose = true, negate = true}`: verbose mode, when the operator
is negated by an enclosing `$not` operator. Returns a list of
`#failure` records.
The different negation modes are needed because, in order to generate
meaningful failure messages, we need to record whether an operator was
negated. The behaviour of combinators like `$and`, `$or`, `$allMatch`
and `$elemMatch` means not all `$not` operators can be normalized out of
the selector before evaluation. Instead, when we encounter a `$not`
during evaluation, we flip the `#ctx.negate` field before evaluating the
inner operator.
Until now, document updates rejected by a Mango VDU returned an opaque "forbidden" message to the client. This commit adds a detailed list of failures, obtained by converting the `#failure` records returned by `mango_selector:match/3` into human-readable messages.
…ct if the doc is newly created
Currently, when a design doc is updated, we validate the `map` and `reduce` fields, but not `validate_doc_update`. Instead, trying to update any other doc while an invalid `validate_doc_update` exists will trigger an error. This comment makes VDU validation more 'eager' by performing it when the ddoc itself is updated. Normal doc writes will still trigger an error if an invalid `validate_doc_update` already exists, but now we try to prevent this happening by validating VDUs when they are first created.
…keyMapMatch") should be considered successful when applied to values of the wrong type
…lements when reporting VDU failures
jcoglan
force-pushed
the
mango-match-failures
branch
from
August 7, 2026 09:18
200cbda to
384cd8e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR has the same content as #5858, it is just in a branch in the
apacherepo to see if this resolves problems with CI.