Make boolean query clause limit configurable per-query [LUCENE-7880]

[asfimport]

As we know, the magic BooleanQuery.maxClauseCount has bitten many people over time.
It's also a static, which really hurts multi-tenancy (i.e. we can't have different settings for different users, clients, or use-cases).

If we want to keep this static as a default, then at least we should allow it to be overridden on a per-query basis when we know it is the desired behavior and not a bug.

Perhaps the simplest way to achieve this would be a setter on BooleanQuery.Builder that configures the limit for that instance only?

---

Migrated from LUCENE-7880 by Yonik Seeley (@yonik), updated Jun 20 2017
Linked issues:

• Raise maxClauseCount in BooleanQuery to Integer.MAX_VALUE [LUCENE-4835] #5900
• SOLR-10921
• SOLR-4586

[asfimport]

David Smiley (@dsmiley) (migrated from JIRA)

+1 fine idea. With this new proposed option, BooleanQuery.maxClauseCount could still exist but only as a default for when you don't go out of your way to set the max on the Builder.

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

Anyone object to the idea of a getter/setter on BooleanQuery.Builder? If not, I can work up a patch.

[asfimport]

Adrien Grand (@jpountz) (migrated from JIRA)

I think of the max clause count as something that should never be changed. What is the use-case for having more than 1024 clauses? It sounds like the user is using a BooleanQuery when he should actually use a TermInSetQuery? I am not a fan of adding a per-BooleanQuery maxClauseCount. Moreover it feels like an unnecessary API to me: users can already check the number of clauses by themselves, which is not harder than calling a setter?

[asfimport]

Erick Erickson (@ErickErickson) (migrated from JIRA)

+1

My first reaction was that burdening the query writer with changing this parameter would be pretty trappy. Then I realized that we could make it a default or invariant either for all request handlers or individual ones. Individual request handlers could even have different values, right?

If that's true, then I'd favor deprecating (and eventually removing support in 8.0) the naked <maxBooleanClauses> from solrconfig. Move it to an initParams or just leave it out of the config completely and have it default in the code to whatever we want if not specified on the query.

Really though any solution that got us around the fact that "last solrconfig maxBooleanClauses loaded wins" trap works for me. +1 to this solution b/c it gives us quite a bit of flexibility without making things trappy.

[asfimport]

Erick Erickson (@ErickErickson) (migrated from JIRA)

Adrien:

We simply cannot create arbitrary limits and say "you should learn to do a better search and not need to ever exceed X" because we cannot anticipate the "meta issues" Solr or Lucene users have to deal with.

Here are some use-cases I've seen in the wild:

I have seen very complex queries in legal situations that run to 20K + raw input before rewrite. Ditto pharma applications. If you're investing 750M in drug development you want to be very, very, very sure that there is no prior art you'd violate. These organizations pay people who create amazingly complex queries and mix-n-match them to insure that they're not infringing on a patent.

I've seen these organizations create 4K reusable sub-queries that they then combine in various ways to accomplish their goals. A dozen at a time. Or more.

It gets worse in the legal arena. I can and have said from an IR standpoint, why are you searching for (runn OR runni OR runnin OR running) Why not just stem? Ans: Because I can defend this search. I cannot explain in court that "we did (or didn't) find document X because the search engine we're using uses a stemming algorithm that did (or didn't) match". And there may be hundreds and hundreds of such clauses.

Genome matching. Believe it or not I've worked with people using Solr/Lucene for matching genome sequences. The complexity here is beyond belief. Machine generated to be sure.

These kinds of domains often do not care how long a search takes. Well, not quite true but for some applications "10 minutes? Great!". I once worked with an organization that had, prior to Solr, a 48 hour turn-around and were happy with anything less than an hour and could live with longer ones upon occasion. Not very many users, true. So my reflexive reaction of "well, these will be expensive queries, it may take some time to return" is invalid.

So you see why I'll -1 imposing limits that aren't absolutely necessary every time.

[asfimport]

Adrien Grand (@jpountz) (migrated from JIRA)

This is not the only arbitrary limit we have. For instance terms can't be greater than 32KB and you can only use edit distances of 1 or 2 with FuzzyQuery.

In some of the cases you mentioned, it seems to me that users should be using TermInSetQuery instead. Maybe a few of them actually need scoring, which is only provided by BooleanQuery. However, to me those are corner cases, and I don't like the idea of extending the API of an important class like BooleanQuery just to help deal with corner-cases.

To me the best option is to keep things as they are today: let users know when they are using BooleanQuery in ways that won't perform well, yet still allow a minority of corner-cases that need greater limits to increase the max clause count through the static setter.

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

users can already check the number of clauses by themselves

That only works to lower the limit, not raise it. But it is more straightforward to raise the static limit and do checking at a higher level, which allows for choices to be made on context. I've proposed this in SOLR-4586

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

This is not the only arbitrary limit we have. For instance terms can't be greater than 32KB and you can only use edit distances of 1 or 2 with FuzzyQuery.

At least some of these upper limits are certainly not arbitrary (and hence not configurable). AFAIK, simply increasing these numbers would not result in working code, hence the limits represent real technical limitations. IIRC, the history behind the 32KB is that the length was packed in an java char (16 bits).

[asfimport]

Erick Erickson (@ErickErickson) (migrated from JIRA)

bq: This is not the only arbitrary limit we have

This is not a justification to impose limits where there's no technical reason to. And imposing a limit on maxbooleanclauses that cannot be changed would take away capabilities that at least some users currently rely on.

bq: In some of the cases you mentioned, it seems to me that users should be using TermInSetQuery instead

Sure. I never claimed that there weren't ways around some/all of the individual cases. My point is there's case N+1 that we haven't thought of and arbitrary, unnecessary limits do not serve the end users well. And taking away current capabilities requires a very good technical reason in my view.

Oh, and I didn't even mention that the queries can be distributed across N fields by edismax.

Yes, yes, yes. That reflects sub-optimal design. User's don't care. Once the performance/functionality is "good enough" in their eyes, whether it could be made better is mostly irrelevant. And forcing them deep into the code in order to do what they're doing now when there's no technical reason to simply creates another barrier they have to overcome. Premature optimization and all that.

[asfimport]

Mark Miller (@markrmiller) (migrated from JIRA)

I'm all for trappy limits that pop up out of the blue because devs know better than me about my resources and use cases and where stuff should just start failing for my own good with runtime exceptions.. I've been meaning to make sure you can only have 5 segments before exceptions are thrown as well. A lot of performance seems to drop off that and users might be get confused without their app breaking at 5. Once I sneak that in, I intend to veto lock it till 2035 when hardware might be up to the task of tearing me from my death veto grip, as which time 6 segments can be the new dev knows when to exception screw you limit.

No wait. I take it back. That was the paranoid inducing vodka talking.

[asfimport]

Adrien Grand (@jpountz) (migrated from JIRA)

Mark, what is your proposal?

This issue looks like a tentative to workaround the fact that there was disagreement about removing the limit entirely in #5900. However adding a per-query API feels even worse to me. It adds an unnecessary method to one of the main classes of our API, and will contaminate many other classes like query parsers, maybe MoreLikeThis, etc. if we want to be able to set different limits on boolean queries that these classes generate too.

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

First, lets assume that having this global limit is a valuable way to catch bugs (I disagree, but let's assume it for the sake of argument).
Now what if someone finds a place in their code where they legitimately need to build a bigger scoring boolean query? What should they do? If they raise the global static limit, they lose this valuable way to catch bugs. If one accepts the first assumption, then it seems like one needs a way to override this limit on a per query basis. If one does not accept the first assumption, then the limit should simply be removed.

Even at a higher granularity, what if one collection wants one limit and a different collection wants another limit (they could even be different customers collocated in the same JVM). How should that be solved? See SOLR-4586 for a more background / arguments.

If you are against the proposal in this issue, then what is your proposal to address these issues Adrien?

[asfimport]

Adrien Grand (@jpountz) (migrated from JIRA)

I think the source of the disagreement is that to me there is almost no good reason to build large boolean queries. So users who really need to do this can increase the limit. They would indeed lose the safety of that limit but it still sounds like a better trade-off to me than removing the max clause count entirely or making it configurable per-query.

As far as multi-tenancy is concerned, it should still be extremely rare that large boolean queries are needed on a per-cluster basis. So I'd make it a per-cluster option and raise the limit if any tenant needs to build large boolean queries.

I understand my assumptions do not seem to match the reality some other committers are seeing based on the comments on SOLR-4586. Also I don't want to prevent Solr from setting a limit equal to MAX_VALUE or making it configurable per-core given my position. So I won't object if you want to follow either of those paths but I still think we should keep things as they are in Lucene since it is our best option from a Lucene perspective.

[asfimport]

David Smiley (@dsmiley) (migrated from JIRA)

By the way, this maxBooleanClauseCount is, I think, extremely similar to maxDeterminizeStates for AutomatonQuery (thus WildCardQuery, RegexpQuery). Both ideas are self-imposed limits that usually don't need to be increased beyond the defaults but cases happen. In both cases, a RuntimeException derivative is thrown if the limit is exceeded. maxDeterminizeStates is configurable for the query instance, and the setting is configurable in the classic query parser's QueryParserBase. maxBooleanClauseCount has neither. Why the difference @jpountz?

[asfimport]

Mark Miller (@markrmiller) (migrated from JIRA)

The difference is probably that people that also work on Solr were not pushing for that. It's easy to get cooperation or an attitude of consensus building in that case. It's also simple to add technically simple enhancements without religious ideas on clearly subjective changes.