Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set analyzer to regex query string search #3967

Merged
merged 11 commits into from
Aug 15, 2022
+32 −7
Merged

Set analyzer to regex query string search #3967

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d2e6e08
set analyzer to regex query string search
yyyogev Jul 21, 2022
036029a
pr changes
yyyogev Jul 24, 2022
64e6098
remove comments
yyyogev Jul 24, 2022
f5351fa
Merge branch 'main' of github.com:/opensearch-project/OpenSearch into…
yyyogev Jul 24, 2022
04fb97f
small fix
yyyogev Jul 24, 2022
b3a4cec
lint
yyyogev Jul 24, 2022
105f68d
small change
yyyogev Jul 25, 2022
f28e335
undo change in wildcard queries to preserve pipeline
yyyogev Jul 26, 2022
930cbd8
Merge branch 'main' of github.com:/opensearch-project/OpenSearch into…
yyyogev Jul 27, 2022
c2ed7a2
Merge branch 'main' of github.com:/opensearch-project/OpenSearch into…
yyyogev Jul 31, 2022
5d40287
spotless
yyyogev Jul 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions rest-api-spec/src/main/resources/rest-api-spec/test/search/190_index_prefix_search.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ setup:
id: 1
body: { text: some short words with a stupendously long one }

- do:
index:
index: test
id: 2
body: { text: sentence with UPPERCASE WORDS }

- do:
indices.refresh:
index: [test]
Expand Down Expand Up @@ -76,6 +82,22 @@ setup:
- match: {hits.max_score: 1}
- match: {hits.hits.0._score: 1}

---
"search with uppercase regex":
- do:
search:
rest_total_hits_as_int: true
index: test
body:
query:
query_string:
default_field: text
query: /UPPERCASE/

- match: {hits.total: 1}
- match: {hits.max_score: 1}
- match: {hits.hits.0._score: 1}

---
"search index prefixes with span_multi":
- do:
Expand Down
32 changes: 19 additions & 13 deletions server/src/main/java/org/opensearch/index/search/QueryStringQueryParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
import org.apache.lucene.search.SynonymQuery;
import org.apache.lucene.search.WildcardQuery;
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.automaton.RegExp;
import org.opensearch.common.lucene.search.Queries;
import org.opensearch.common.regex.Regex;
import org.opensearch.common.unit.Fuzziness;
Expand Down Expand Up @@ -717,20 +716,25 @@ private Query getWildcardQuerySingle(String field, String termStr) throws ParseE
// effectively, we check if a field exists or not
return existsQuery(field);
}
String indexedNameField = field;
Analyzer oldAnalyzer = getAnalyzer();
try {
MappedFieldType currentFieldType = queryBuilder.context.fieldMapper(field);
if (currentFieldType == null) {
return newUnmappedFieldQuery(field);
}
if (forceAnalyzer != null && (analyzeWildcard || currentFieldType.getTextSearchInfo().isTokenized())) {
setAnalyzer(forceAnalyzer);
return super.getWildcardQuery(currentFieldType.name(), termStr);
if (currentFieldType != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm ... did we lost if (currentFieldType == null) { condition?

Copy link
Contributor Author

@yyyogev yyyogev Jul 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured it will be cleaner to check if (currentFieldType != null) now that we always set the analyzer

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is unmapped field case, we probably should return newUnmappedFieldQuery ?

// return newUnmappedFieldQuery(field);
setAnalyzer(forceAnalyzer == null ? queryBuilder.context.getSearchAnalyzer(currentFieldType) : forceAnalyzer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we do this in multiple places maybe add a private getSearchAnalyzer(field) method?

if/else would probably look cleaner

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

indexedNameField = currentFieldType.name();
}

// if (forceAnalyzer != null && (analyzeWildcard || currentFieldType.getTextSearchInfo().isTokenized())) {
// setAnalyzer(forceAnalyzer);
// return super.getWildcardQuery(currentFieldType.name(), termStr);
// }
yyyogev marked this conversation as resolved.
Show resolved Hide resolved
if (getAllowLeadingWildcard() == false && (termStr.startsWith("*") || termStr.startsWith("?"))) {
throw new ParseException("'*' or '?' not allowed as first character in WildcardQuery");
}
return currentFieldType.wildcardQuery(termStr, getMultiTermRewriteMethod(), context);
return super.getWildcardQuery(indexedNameField, termStr);
// return currentFieldType.wildcardQuery(termStr, getMultiTermRewriteMethod(), context);
} catch (RuntimeException e) {
if (lenient) {
return newLenientFieldQuery(field, e);
Expand Down Expand Up @@ -781,11 +785,13 @@ private Query getRegexpQuerySingle(String field, String termStr) throws ParseExc
if (currentFieldType == null) {
return newUnmappedFieldQuery(field);
}
if (forceAnalyzer != null) {
setAnalyzer(forceAnalyzer);
return super.getRegexpQuery(field, termStr);
}
return currentFieldType.regexpQuery(termStr, RegExp.ALL, 0, getDeterminizeWorkLimit(), getMultiTermRewriteMethod(), context);
setAnalyzer(forceAnalyzer == null ? queryBuilder.context.getSearchAnalyzer(currentFieldType) : forceAnalyzer);
return super.getRegexpQuery(field, termStr);
// if (forceAnalyzer != null) {
// setAnalyzer(forceAnalyzer);
// return super.getRegexpQuery(field, termStr);
// }
// return currentFieldType.regexpQuery(termStr, RegExp.ALL, 0, getDeterminizeWorkLimit(), getMultiTermRewriteMethod(), context);
} catch (RuntimeException e) {
if (lenient) {
return newLenientFieldQuery(field, e);
Expand Down