Skip to content

Commit

Permalink
Fix corner case in REPEAT_MIN_MAX and clean up code
Browse files Browse the repository at this point in the history
If a regex has e.g. `(foo){0,5}`, we shouldn't treat the expression as
required. Instead, we should throw our hands in the air and return
MatchAllDocs.

Also, I realized that you can only reach the end of regexpToQuery if you
have a BooleanQuery, so there's no need to type-check and cast before
collapsing the single-clause case.

Signed-off-by: Michael Froh <[email protected]>
  • Loading branch information
msfroh committed May 30, 2024
1 parent 666caae commit d6bdc7c
Showing 1 changed file with 8 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ public Query regexpQuery(
* @return a query that matches on the known required parts of the given regular expression
*/
private static Query regexpToQuery(String fieldName, RegExp regExp) {
Query query;
BooleanQuery query;
if (Objects.requireNonNull(regExp.kind) == RegExp.Kind.REGEXP_UNION) {
List<Query> clauses = new ArrayList<>();
while (regExp.exp1.kind == RegExp.Kind.REGEXP_UNION) {
Expand Down Expand Up @@ -569,16 +569,14 @@ private static Query regexpToQuery(String fieldName, RegExp regExp) {
}
}
query = builder.build();
} else if (regExp.kind == RegExp.Kind.REGEXP_REPEAT_MIN || regExp.kind == RegExp.Kind.REGEXP_REPEAT_MINMAX) {
return regexpToQuery(fieldName, regExp.exp1);
} else {
return new MatchAllDocsQuery();
}
if (query instanceof BooleanQuery) {
BooleanQuery booleanQuery = (BooleanQuery) query;
if (booleanQuery.clauses().size() == 1) {
return booleanQuery.iterator().next().getQuery();
} else if ((regExp.kind == RegExp.Kind.REGEXP_REPEAT_MIN || regExp.kind == RegExp.Kind.REGEXP_REPEAT_MINMAX)
&& regExp.min > 0) {
return regexpToQuery(fieldName, regExp.exp1);

Check warning on line 574 in server/src/main/java/org/opensearch/index/mapper/WildcardFieldMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/WildcardFieldMapper.java#L574

Added line #L574 was not covered by tests
} else {
return new MatchAllDocsQuery();
}
if (query.clauses().size() == 1) {
return query.iterator().next().getQuery();
}
return query;
}
Expand Down

0 comments on commit d6bdc7c

Please sign in to comment.