lucene icon indicating copy to clipboard operation
lucene copied to clipboard

Support disabling IndexSearcher.maxClauseCount with a value of -1

Open dsmiley opened this issue 1 year ago • 2 comments

This PR allows IndexSearcher.maxClauseCount to be -1 with the meaning of there being no limit. This avoids pointless query tree visiting. Tangential:TermInSetQuery.visit() is disappointing.

dsmiley avatar Mar 12 '24 13:03 dsmiley

I'm curious what problem you are trying to address. It looks like you're trying to avoid the overhead of checking the number of clauses, but intuitively this wouldn't help much as we have other operations that need to run in linear time with the number of clauses in the query anyway such as creating the query in the first place, rewriting it, creating a weight, etc.

jpountz avatar Mar 13 '24 08:03 jpountz

Problem: clarity in being able to turn it off; a -1 value would clarify the configuration intent. It has become even less useful, not that I quite wanted it in the first place in the past either. In the production clusters I work with, there are other limits (query parsing level enforced) and they are adequate. I could use a bunch of 9's and add a comment in the configuration but shouldn't Lucene let you turn it off? Why argue against a simple if condition for a check the user doesn't want to enforce. I understand your point that it probably costs pathetically little compared to executing the query anyway. However note the sad TermInSetQuery.visit() implementation.

Additionally/alternatively, making getNumClausesCheckVisitor protected and non-static could be useful.

Additionally/alternatively, remove this limit. Keep a sample QueryVisitor impl with instructional javadocs to show how trivial it is for a user to layer this on if they so choose (subclass rewrite)

dsmiley avatar Mar 14 '24 17:03 dsmiley