LUCENE-9445 Add support for case insensitive regex searches in QueryParser

[markharwood]

This PR uses the standard /.../i regex syntax to denote case insensitive queries, exposing the underlying case insensitive regex support added in LUCENE-9386

This could be considered a breaking change if users had a regex immediately followed by the letter i but I imagine a case insensitive search would have been the intention of the searcher all along.

Jira issue: https://issues.apache.org/jira/browse/LUCENE-9445

[markharwood]

I wonder if the parsing should be more strict and only matches if there is a separator or ends after the i ?

Yep - end-of-string or space should be required after the end of the regex. Maybe brackets too for Boolean logic? You're right that this means a breaking change. I just tested range queries ie num:[1 TO 5]i and that tolerates no whitespace between the closing ] and a search for the letter i so it's currently legal syntax (although weird and probably rare).

I hadn't considered adding a flag for 8.x. If we do I'd prefer to see it support the new behaviour by default - the rationale being that it is better to give an escape hatch to the few admins concerned about BWC for an edge case than continue the legacy of silent failures for potentially many regex searchers assuming /Foo/i was supported syntax.

[markharwood]

Progress update - I'm struggling a bit with how to make the parser stricter i.e. ensuring there's a space between /Foo/i and the next search term. I'd also need to allow for ( A OR /Foo/i) with the closing bracket instead of a space. I'm not yet clear in JavaCC how to write that rule without also consuming the closing bracket. Some BWC issues to work through here too.

[markharwood]

I wonder if the parsing should be more strict and only matches if there is a separator or ends after the i ?

@jimczi What is the behaviour for a non-match? Should /regex/iterm throw an error or be interpreted as /regex/ and iterm?

[jimczi]

Should /regex/iterm throw an error or be interpreted as /regex/ and iterm?

I'd prefer that we throw an error if any of the character attached to a regexp is not recognized.

[markharwood]

OK. @romseygeek suggested the BWC flag is called "allow_modifiers" and, if false, legacy behaviour is used ie there would be no errors for characters after trailing / and /regex/i is still interpreted as /regex/ and term i.

[markharwood]

@jimczi The TL/DR is I think it's going to be too hard to implement the stricter parsing logic.

I spoke with @romseygeek and we couldn't see a neat way that the string after the closing / could be processed. The parser doesn't allow us to write a handler that behaves differently depending on a flag:

Eager option - pass everything immediately after closing / to the regexp constructor logic.

1. When in strict mode , a simple non-i string is easy to detect and error on.
2. When not in strict mode the non-i string has to then be parsed as a regular clause (along with any boosts). It's not possible for the regex query-building logic to invoke the parser to do this.

Lazy option - pass only any trailing i to the regexp constructor logic

This simplifies the regex construction logic but pushes the problem elsewhere - in strict mode, the next clause the parser builds would have to double-check it wasn't immediately preceded by a regex with no whitespace to separate them and throw an error. The parsing framework does not provide such a context that would allow this logic.

Alternative option

We did consider forgetting about /Foo/i syntax and opting for the inline syntax commonly supported by regex parsers e.g. /(?i)Foo/ but this is less well known and more cryptic.

Summary

We felt that the existing lax implementation where /Foo/iphone is interpreted as /[Ff][Oo][Oo]/ OR phone is not so awful that it warrants the added complexity needed to make it work as we'd ideally want. If there are any smart ideas we're open to them but it looks like a very messy problem.