Case-insensitive TermInSetQuery Implementation (Proof of Concept) - #14349
Case-insensitive TermInSetQuery Implementation (Proof of Concept)#14349willdickerson wants to merge 6 commits into
Conversation
This commit adds a proof of concept implementation for a case-insensitive variant of TermInSetQuery. Key features: - Uses regex-based pattern matching with case-insensitive flag - Implements automaton-based matching for terms - Includes comprehensive test cases - Handles special cases like locale considerations
| * A specialized TermsEnum implementation that performs case-insensitive matching against the | ||
| * terms dictionary using a ByteRunAutomaton for proper Unicode case handling. | ||
| */ | ||
| private class CaseInsensitiveSetEnum extends FilteredTermsEnum { |
There was a problem hiding this comment.
Don't do this, you lose all skipping and it will evaluate every single term in the terms dictionary. extremely slow. just extend Query and rewrite() to RegexpQuery since that's what you are doing.
| // Ensure the automaton is deterministic | ||
| if (!automaton.isDeterministic()) { | ||
| automaton = Operations.determinize(automaton, Operations.DEFAULT_DETERMINIZE_WORK_LIMIT); | ||
| } |
There was a problem hiding this comment.
The automaton here is probably on the order of excessive if you have many terms. try it with a few thousand
…sitiveTermInSetQuery Fixes based on maintainer feedback: - Replaced FilteredTermsEnum with a RegexpQuery-based implementation - Ensures proper deterministic automaton handling - Updates tests to accommodate implementation details - Fixes imports and linting issues
| // Test with a random subset of terms | ||
| List<String> queryBaseTerms = new ArrayList<>(baseTerms); | ||
| queryBaseTerms = queryBaseTerms.subList(0, Math.min(5, queryBaseTerms.size())); | ||
|
|
There was a problem hiding this comment.
I think you'd need to test this with thousands of terms -- 5 isn't going to be enough to detect issues. (It's small enough that you could just do a disjunction of case-insensitive term queries.)
I like @rmuir's suggestion on the mailing list that a case-insensitive version of Automata.makeStringUnion might be reasonable.
There was a problem hiding this comment.
I've added a test case with 5,000 terms in testLargeNumberOfTerms(). Instead of using Automata.makeStringUnion, I went with a HashSet-based approach that avoids the automaton determinization costs. I can explore the Automata.makeStringUnion approach if you prefer that direction.
This commit addresses the maintainer's comment about automaton inefficiency with many terms by:
1. Using a HashSet-based approach instead of determinizing a large automaton
2. Creating a simple placeholder automaton for large term sets
3. Implementing threshold-based switching between strategies:
- RegexpQuery for single terms
- BooleanQuery for smaller sets (<= 100 terms)
- AutomatonQuery with HashSet lookup for large sets
| public boolean run(byte[] s, int offset, int length) { | ||
| // Convert input bytes to string and lowercase for comparison | ||
| BytesRef slice = new BytesRef(s, offset, length); | ||
| String termString = slice.utf8ToString().toLowerCase(); | ||
| return lowerCaseTerms.contains(termString); | ||
| } |
There was a problem hiding this comment.
The ByteRunAutomaton is only used for highlighting (see MultiTermHighlighting). The actual query will use the Automaton passed to AutomatonQuery.
| private Automaton createAutomaton() { | ||
| // Create a minimal automaton that accepts any string | ||
| // The actual matching is done via the ByteRunAutomaton override | ||
| Automaton automaton = new Automaton(); | ||
| int state = automaton.createState(); | ||
| automaton.setAccept(state, true); | ||
| return automaton; | ||
| } |
There was a problem hiding this comment.
This Automaton will only match the empty string. (It's equivalent to Automata.makeEmptyString().)
I believe the query will be very fast, but won't match any terms (except the empty string). In the end, I believe we'd need an Automaton that matches all the strings in a case-insensitive fashion.
I think it would be possible to do that with some changes to the StringsToAutomaton class to support case-insensitive transitions. Essentially, for each codepoint, we would need to add a transition for the lower- and upper-case versions (if applicable) in the convert method.
|
@willdickerson -- I took a stab at modifying |
|
In my head, that's what we need. There is a crazy difference in construction and execution time between a "naive" union and using the efficient linear-time algorithm, as opposed to going worst-case exponential. With simple case-folding it may still be possible to do this though. Try using the new test functions available such as edit: autocorrect-correct |
|
Implementing it as a query that rewrites to the proper query based on the list of terms makes sense to me. You may want to move this query to the lucene-queries module since it doesn't really qualify as a "core" query. |
|
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution! |
|
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution! |
Overview
This PR introduces a proof of concept for a case-insensitive variant of TermInSetQuery. The implementation provides an efficient way to search for terms regardless of case without needing to generate all case variations.
Implementation Details
MultiTermQueryto leverage existing infrastructureByteRunAutomatonfor efficient matchingLimitations
Testing
The implementation includes comprehensive test cases covering:
This POC is intended to gather feedback from the community