Fix invalid flag setting for RegExp

[iverase]

This test failure is a side effect of the upgrade to a new Lucene snapshot

relates #61957

[elasticmachine]

Pinging @elastic/es-search (:Search/Search)

[jimczi]

I think the change is ok but I wonder why that failed the test. We're supposed to rewrite the old all transparently.
@markharwood can you check that the leniency is applied correctly ?

[iverase]

There is a change that was not in the current form: apache/lucene-solr#1659

[nik9000]

There are a few problems with regexes in runtime fields I discovered yesterday. One is that they don't use the case insensitive infrastructure mark added while the branch was open. That is probably part of what is up here.

…

On Fri, Sep 4, 2020, 07:29 Ignacio Vera ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/StringScriptFieldRegexpQueryTests.java <#61976 (comment)> : > @@ -59,7 +59,7 @@ protected StringScriptFieldRegexpQuery mutate(StringScriptFieldRegexpQuery orig) pattern += "modified"; break; case 3: - flags = randomValueOtherThan(flags, () -> randomInt(0xFFFF)); + flags = randomValueOtherThan(flags, () -> randomInt(RegExp.ALL)); There is a change that was not in the current form: apache/lucene-solr#1659 <apache/lucene-solr#1659> — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#61976 (comment)>, or unsubscribe <https:/notifications/unsubscribe-auth/AABUXIXQ5KJSCI2GWWAYLXTSEDFTNANCNFSM4QYB2WBA> .

[iverase]

Thanks @nik9000 , that makes sense. When adding case insensitivity, the check for the flag range was added.

[markharwood]

LGTM- The change from OxFFFF to OxFF (RegExp.ALL) is what I would expect a well-behaved client to do now.
I'll need to look deeper into what the (lack of) leniency for bad flags was going on here.

Related - I'm poking around in the scripted stuff just now as part of a more general case insensitivity PR for term/terms/prefix.
In that PR I note that AbstractScriptMappedFieldType deviated from my intended changes to the base class (adding a case insensitive flag to term/terms/wildcard/prefix query) because it declares the old method signature but is missing the override annotation. I can clean up in my PR or leave it a separate issue?

[nik9000]

I have an open PR that adds the override annotation. It's bundled with some loop detection fix because that is when I noticed it. On mobile so I can't find it. I really wish we could force that sort of thing with checkstyle. The lack of override annotation means we didn't have regexp query support on runtime keywords at all. If your PR also adds the override annotations it'll merge conflict but that is super ok. Better twice than never.

…

On Fri, Sep 4, 2020, 07:55 markharwood ***@***.***> wrote: LGTM- The change from OxFFFF to OxFF (RegExp.ALL) is what I would expect a well-behaved client to do now. I'll need to look deeper into what the (lack of) leniency for bad flags was going on here. Related - I'm poking around in the scripted stuff just now as part of a more general case insensitivity PR <#61596> for term/terms/prefix. In that PR I note that AbstractScriptMappedFieldType deviated from my intended changes to the base class (adding a case insensitive flag to term/terms/wildcard/prefix query) because it declares the old method signature but is missing the override annotation. I can clean up in my PR or leave it a separate issue? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#61976 (comment)>, or unsubscribe <https:/notifications/unsubscribe-auth/AABUXITJGJIJ6QQ36I7GIYDSEDITFANCNFSM4QYB2WBA> .