fix empty queries #1783
fix empty queries #1783
Conversation
|
I'll defer to others to comment on the Erlang side. Regarding the tests, similar to #1719 it would be good to see tests which verify the results returned are the same with/without an index present. |
| match({[{<<"$or">>, Args}]}, Value, Cmp) -> | ||
| Pred = fun(SubSel) -> match(SubSel, Value, Cmp) end, | ||
| lists:any(Pred, Args); | ||
|
|
||
| match({[{<<"$not">>, Arg}]}, Value, Cmp) -> | ||
| not match(Arg, Value, Cmp); | ||
|
|
||
| match({[{<<"$all">>, []}]}, _, _) -> | ||
| true; |
There was a problem hiding this comment.
Is this a duplicate clause from L404?
There was a problem hiding this comment.
404 is $and, this is $all
There was a problem hiding this comment.
Also of note, a duplicate clause would have triggered a compilation error.
src/mango/test/06-basic-text-test.py
Outdated
| assert len(docs) == 1 | ||
|
|
||
| def test_empty_arrays_complex(self): | ||
| resp = self.db.find({"$or": [], "a": {"$in" : []}}, explain=True) |
There was a problem hiding this comment.
"simple" combinations of empty queries will also short-circuit to no execution right?
|
Is there a fix for this going in anytime soon? |
|
Hi @michaelbreslin , probably not before the holiday break at this point - but I expect it'll land shortly thereafter. Looks like we may need some more tests before this can be merged. |
|
@willholley : Your additional testing suggestion revealed a conflicting issue with special/view indexes vs text indexes. Special/View - We do a full range scan, then filter. I think the solution is to make special/view indexes ignore the query(not execute the fabric call) as well. But it get's a little ugly as the _explain endpoint needs to reflect this as well. |
|
@tonysun83 is the text index behaviour just an optimisation or does it generate an error? If it's just an optimisation then I agree we should be consistent and apply that at a higher level regardless of the index type. If you're proposing generating an error where Mango would previously return an empty result set then I think it needs more discussion as it would be a breaking change. |
|
@willholley : It doesn't generate an error and will be an improvement. My only concern is that it will break existing functionality. I have a test suite for that so that shouldn't be a problem. The issue I'm currently running into is getting all 3 to produce the same result. Ex: produce different results in json, _all_docs, and text depending on where part of the code is implemented. The correct solution should be IMO should be
for all 3 index types. I'm debugging why it's currently not the case |
|
Full regression + new tests passed. I kept the existing functionality intact. |
df3039a
to
8b5295d
Compare
002dc81
to
a93d7a6
Compare
|
After fixing the linter errors and allowing for CI flakes in view_compaction.js, this now fails with an error in the new tests: @tonysun83 do you have time to look into this? |
|
I would like to continue, and found that |
Mango text indexes currently throw function clauses when queries in two
situations:
1) Empty Selector
2) Operators with empty arrays.
We fix this issue and change the behavior as follows:
1) Any empty selector will fall back on _all_docs (like json indexes).
2) $or, $and, $in, $all that specify an empty arrays
will be treated as no-op when combined with other operators.
3) A single no-nop query such as {"$or": []} will not be executed and
return nothing just like json indexes.
4) $nin with an empty array will return all docs that match the field,
just like json indexes.
9ea43f5
to
f606dbd
Compare
e7a5e73
to
bf6b984
Compare
Previously, in #1783, the logic was wrong in relation to how certain operators interacted with empty arrays. We modify this logic to make it such that: {"foo":"bar", "bar":{"$in":[]}} and {"foo":"bar", "bar":{"$all":[]}} should return 0 results.
Previously, in #1783, the logic was wrong in relation to how certain operators interacted with empty arrays. We modify this logic to make it such that: {"foo":"bar", "bar":{"$in":[]}} and {"foo":"bar", "bar":{"$all":[]}} should return 0 results.
Previously, in #1783, the logic was wrong in relation to how certain operators interacted with empty arrays. We modify this logic to make it such that: {"foo":"bar", "bar":{"$in":[]}} and {"foo":"bar", "bar":{"$all":[]}} should return 0 results.
Previously, in #1783, the logic was wrong in relation to how certain operators interacted with empty arrays. We modify this logic to make it such that: {"foo":"bar", "bar":{"$in":[]}} and {"foo":"bar", "bar":{"$all":[]}} should return 0 results.
Previously, in #1783, the logic was wrong in relation to how certain operators interacted with empty arrays. We modify this logic to make it such that: {"foo":"bar", "bar":{"$in":[]}} and {"foo":"bar", "bar":{"$all":[]}} should return 0 results.
Previously, in #1783, the logic was wrong in relation to how certain operators interacted with empty arrays. We modify this logic to make it such that: {"foo":"bar", "bar":{"$in":[]}} and {"foo":"bar", "bar":{"$all":[]}} should return 0 results.
Previously, in #1783, the logic was wrong in relation to how certain operators interacted with empty arrays. We modify this logic to make it such that: {"foo":"bar", "bar":{"$in":[]}} and {"foo":"bar", "bar":{"$all":[]}} should return 0 results.
Previously, in #1783, the logic was wrong in relation to how certain operators interacted with empty arrays. We modify this logic to make it such that: {"foo":"bar", "bar":{"$in":[]}} and {"foo":"bar", "bar":{"$all":[]}} should return 0 results. Co-authored-by: Joan Touzet <[email protected]>
Previously, in #1783, the logic was wrong in relation to how certain operators interacted with empty arrays. We modify this logic to make it such that: {"foo":"bar", "bar":{"$in":[]}} and {"foo":"bar", "bar":{"$all":[]}} should return 0 results. Co-authored-by: Joan Touzet <[email protected]>
Overview
Mango text indexes currently throw function clauses when queries in two
situations:
We fix this issue and change the behavior as follows:
will be treated as no-op when combined with other operators.
return nothing just like json indexes.
just like json indexes.
Testing recommendations
New tests were added to the python suite and can be run with
MANGO_TEXT_INDEXES=1 nosetests test/21-empty-selector-tests.py
This requires a clouseau setup.
Related Issues or Pull Requests
The issue is this PR.
Checklist