[JAY-678] Allow nested Elasticsearch boolean queries

[sergio-bobillier]

Changes the QueryBuilder::QueryClauses::Bool class to allow it to accept nested boolean queries.

The main driver behind this change is the need to combine must and should clauses with a particular precedence when querying multiple indexes, especially when some of the fields in the query exist in only a subset of those indexes.

[sheputis]

question, does this leave the 'not' behaviour consistent? it does not break anything?

[sergio-bobillier]

question, does this leave the 'not' behaviour consistent? it does not break anything?

Well, the negation works by wrapping whatever you currently have into a bool with a single must_not subclause. It doesn't care what's inside. That basically flips around whatever is inside, including nested booleans. Regarding the "consistency" of the resulting clause, well I cannot say for sure that depends on how Elasticsearch interprets the resulting clause.

But that is probably beyond the scope of Jay API. I.e. it cannot be stuffed with extra logic to fix the inconsistencies of Elasticsearch (if there are actually any).

[sheputis]

question, does this leave the 'not' behaviour consistent? it does not break anything?

Well, the negation works by wrapping whatever you currently have into a bool with a single must_not subclause. It doesn't care what's inside. That basically flips around whatever is inside, including nested booleans. Regarding the "consistency" of the resulting clause, well I cannot say for sure that depends on how Elasticsearch interprets the resulting clause.

But that is probably beyond the scope of Jay API. I.e. it cannot be stuffed with extra logic to fix the inconsistencies of Elasticsearch (if there are actually any).

Ah, just to clarify — I didn’t mean for you to verify Elasticsearch’s behavior :)
I was referring specifically to Jay API.

I always forget the exact details of how the query builder is structured (it’s a bit complex), and I remember we’ve had regressions in the not handling before. So my question was just to double-check whether you had that in mind when making the change.

[sergio-bobillier]

I always forget the exact details of how the query builder is structured (it’s a bit complex), and I remember we’ve had regressions in the not handling before. So my question was just to double-check whether you had that in mind when making the change.

No worries. I appreciate you raising the question. I actually went into the code to check what was actually happening. It did not occur to me that the negation could break. So thanks for bringing that up. 🤝