ddelemeny
commented
6 months ago I put parentheses by default to cover a corner case, but with further tests, I realize that it won't work either.
Query modifications add positive or negative occurrence filters to an existing query. The parser does not generally accept mixing occurrence operators and binary boolean operators, so if the initial query has boolean operators, it will fail.
Example :
-> foo AND bar // works
-> foo AND bar -baz // adding a negative filter fails
There's a corner case in the parser implementation that allows mixing operators, that is where explicit boolean binary operations are wrapped in parentheses
-> (foo AND bar) -baz // works?!
But it's unfortunately not just about isolating incompatible operations behind parentheses, the following fails:
-> foo AND bar (-baz) //bummer
Implicit and explicit operators are not consistent either
-> foo AND bar AND (-baz) // works??!?!
However, using the explicit boolean binary operator on top of an existing query with an occurence operator will fail:
-> foo -bar //works
-> foo -bar AND -baz //fails
So... Yeah, parens in this case were not applied where it matters. The only way to produce a reliably correct query for the parser is to isolate the initial query from the appended filter, which implies one of these two :
- rewriting the initial query and making sure parentheses don't pile up
- accepting the query to become ridden with parens with heavy use of the filter actions.
I have strong opinions against tampering with user input as a side effect of another action, there's no good solution here but to wait a fix from upstream.
Our use of the default_operator parameter in the query allows us to rewrite ADD_FILTER operations without the "AND" operator. This, with the use of parentheses, should prevent producing queries that Quickwit can't parse.