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

Extract all used fields from QueryStringQueryBuilder #6020

Closed
wants to merge 4 commits into from
+142 −7
Closed

Extract all used fields from QueryStringQueryBuilder #6020

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d155918
initial commit
petardz Jan 26, 2023
411cc38
various fixes
petardz Jan 26, 2023
4a48124
added unit tests
petardz Jan 26, 2023
2616db2
spotless check fix; changelog
petardz Jan 27, 2023
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
Prev Previous commit
Next Next commit
various fixes 
Signed-off-by: Petar Dzepina <petar.dzepina@gmail.com>
  • Loading branch information
@petardz
petardz authored and Petar Dzepina committed Jan 22, 2024
commit 411cc381538bc93886e8567cea32203d2706305b
2 changes: 0 additions & 2 deletions server/src/main/java/org/opensearch/index/query/QueryStringQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,6 @@ public class QueryStringQueryBuilder extends AbstractQueryBuilder<QueryStringQue

private ZoneId timeZone;

List<String> discoveredFields;

/** To limit effort spent determinizing regexp queries. */
private int maxDeterminizedStates = DEFAULT_DETERMINIZE_WORK_LIMIT;

Expand Down
50 changes: 24 additions & 26 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 @@ -331,13 +331,12 @@ public Query getFieldQuery(String field, String queryText, boolean quoted) throw
return getFieldQuery(field, queryText, getPhraseSlop());
}

onDiscoveredField(field);

// Detects additional operators '<', '<=', '>', '>=' to handle range query with one side unbounded.
// It is required to use a prefix field operator to enable the detection since they are not treated
// as logical operator by the query parser (e.g. age:>=10).
if (field != null) {
if (queryText.length() > 1) {
addDiscoveredField(field);
if (queryText.charAt(0) == '>') {
if (queryText.length() > 2) {
if (queryText.charAt(1) == '=') {
Expand Down Expand Up @@ -368,7 +367,7 @@ public Query getFieldQuery(String field, String queryText, boolean quoted) throw
return newUnmappedFieldQuery(field);
}
for (Map.Entry<String, Float> entry : fields.entrySet()) {
onDiscoveredField(entry.getKey());
addDiscoveredField(entry.getKey());
}
Analyzer oldAnalyzer = queryBuilder.analyzer;
try {
Expand All @@ -383,7 +382,7 @@ public Query getFieldQuery(String field, String queryText, boolean quoted) throw
}
}

private void onDiscoveredField(String field) {
private void addDiscoveredField(String field) {
Copy link
Member

Choose a reason for hiding this comment

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

if we have to keep invoke addDiscoveredField in every method manually we can't guarantee that this wont be missed in any methods added in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That could be an issue.

The only alternative I can think of is maybe traversing resulting Query "tree", but there also we could add new types of queries and miss some fields..

if (field == null || Regex.isSimpleMatchPattern(field)) {
return;
}
Expand All @@ -400,9 +399,6 @@ protected Query getFieldQuery(String field, String queryText, int slop) throws P
if (fields.isEmpty()) {
return newUnmappedFieldQuery(field);
}
for (Map.Entry<String, Float> entry : fields.entrySet()) {
onDiscoveredField(entry.getKey());
}
Analyzer oldAnalyzer = queryBuilder.analyzer;
int oldSlop = queryBuilder.phraseSlop;
try {
Expand All @@ -416,6 +412,9 @@ protected Query getFieldQuery(String field, String queryText, int slop) throws P
if (query == null) {
return null;
}
for (Map.Entry<String, Float> entry : fields.entrySet()) {
addDiscoveredField(entry.getKey());
}
return applySlop(query, slop);
} catch (IOException e) {
throw new ParseException(e.getMessage());
Expand All @@ -442,7 +441,7 @@ protected Query getRangeQuery(String field, String part1, String part2, boolean

List<Query> queries = new ArrayList<>();
for (Map.Entry<String, Float> entry : fields.entrySet()) {
onDiscoveredField(entry.getKey());
addDiscoveredField(entry.getKey());
Query q = getRangeQuerySingle(entry.getKey(), part1, part2, startInclusive, endInclusive, context);
assert q != null;
queries.add(applyBoost(q, entry.getValue()));
Expand All @@ -468,7 +467,7 @@ private Query getRangeQuerySingle(
return newUnmappedFieldQuery(field);
}
try {
onDiscoveredField(currentFieldType.name());
addDiscoveredField(currentFieldType.name());
Analyzer normalizer = forceAnalyzer == null ? queryBuilder.context.getSearchAnalyzer(currentFieldType) : forceAnalyzer;
BytesRef part1Binary = part1 == null ? null : normalizer.normalize(field, part1);
BytesRef part2Binary = part2 == null ? null : normalizer.normalize(field, part2);
Expand All @@ -493,7 +492,6 @@ private Query getRangeQuerySingle(

@Override
protected Query handleBareFuzzy(String field, Token fuzzySlop, String termImage) throws ParseException {
onDiscoveredField(field);
if (fuzzySlop.image.length() == 1) {
return getFuzzyQuery(field, termImage, fuzziness.asDistance(termImage));
}
Expand All @@ -509,7 +507,7 @@ protected Query getFuzzyQuery(String field, String termStr, float minSimilarity)
}
List<Query> queries = new ArrayList<>();
for (Map.Entry<String, Float> entry : fields.entrySet()) {
onDiscoveredField(entry.getKey());
addDiscoveredField(entry.getKey());
Query q = getFuzzyQuerySingle(entry.getKey(), termStr, minSimilarity);
assert q != null;
queries.add(applyBoost(q, entry.getValue()));
Expand All @@ -529,7 +527,7 @@ private Query getFuzzyQuerySingle(String field, String termStr, float minSimilar
return newUnmappedFieldQuery(field);
}
try {
onDiscoveredField(field);
addDiscoveredField(field);
Analyzer normalizer = forceAnalyzer == null ? queryBuilder.context.getSearchAnalyzer(currentFieldType) : forceAnalyzer;
BytesRef term = termStr == null ? null : normalizer.normalize(field, termStr);
return currentFieldType.fuzzyQuery(
Expand All @@ -550,7 +548,7 @@ private Query getFuzzyQuerySingle(String field, String termStr, float minSimilar

@Override
protected Query newFuzzyQuery(Term term, float minimumSimilarity, int prefixLength) {
onDiscoveredField(term.field());
addDiscoveredField(term.field());
int numEdits = Fuzziness.build(minimumSimilarity).asDistance(term.text());
if (fuzzyRewriteMethod != null) {
return new FuzzyQuery(term, numEdits, prefixLength, fuzzyMaxExpansions, fuzzyTranspositions, fuzzyRewriteMethod);
Expand All @@ -567,7 +565,6 @@ protected Query getPrefixQuery(String field, String termStr) throws ParseExcepti
}
List<Query> queries = new ArrayList<>();
for (Map.Entry<String, Float> entry : fields.entrySet()) {
onDiscoveredField(entry.getKey());
Query q = getPrefixQuerySingle(entry.getKey(), termStr);
if (q != null) {
queries.add(applyBoost(q, entry.getValue()));
Expand All @@ -590,7 +587,7 @@ private Query getPrefixQuerySingle(String field, String termStr) throws ParseExc
if (currentFieldType == null || currentFieldType.getTextSearchInfo() == TextSearchInfo.NONE) {
return newUnmappedFieldQuery(field);
}
onDiscoveredField(field);
addDiscoveredField(field);
setAnalyzer(getSearchAnalyzer(currentFieldType));
Query query = null;
if (currentFieldType.getTextSearchInfo().isTokenized() == false) {
Expand All @@ -611,6 +608,7 @@ private Query getPrefixQuerySingle(String field, String termStr) throws ParseExc

private Query getPossiblyAnalyzedPrefixQuery(String field, String termStr, MappedFieldType currentFieldType) throws ParseException {
if (analyzeWildcard == false) {
addDiscoveredField(currentFieldType.name());
return currentFieldType.prefixQuery(
getAnalyzer().normalize(field, termStr).utf8ToString(),
getMultiTermRewriteMethod(),
Expand All @@ -625,7 +623,7 @@ private Query getPossiblyAnalyzedPrefixQuery(String field, String termStr, Mappe
source = getAnalyzer().tokenStream(field, termStr);
source.reset();
} catch (IOException e) {
onDiscoveredField(field);
addDiscoveredField(field);
return super.getPrefixQuery(field, termStr);
}
tlist = new ArrayList<>();
Expand Down Expand Up @@ -659,6 +657,7 @@ private Query getPossiblyAnalyzedPrefixQuery(String field, String termStr, Mappe
}

if (tlist.size() == 1 && tlist.get(0).size() == 1) {
addDiscoveredField(currentFieldType.name());
return currentFieldType.prefixQuery(tlist.get(0).get(0), getMultiTermRewriteMethod(), context);
}

Expand All @@ -670,24 +669,24 @@ private Query getPossiblyAnalyzedPrefixQuery(String field, String termStr, Mappe
Query posQuery;
if (plist.size() == 1) {
if (isLastPos) {
onDiscoveredField(currentFieldType.name());
addDiscoveredField(currentFieldType.name());
posQuery = currentFieldType.prefixQuery(plist.get(0), getMultiTermRewriteMethod(), context);
} else {
onDiscoveredField(field);
addDiscoveredField(field);
posQuery = newTermQuery(new Term(field, plist.get(0)), BoostAttribute.DEFAULT_BOOST);
}
} else if (isLastPos == false) {
// build a synonym query for terms in the same position.
SynonymQuery.Builder sb = new SynonymQuery.Builder(field);
addDiscoveredField(field);
for (String synonym : plist) {
onDiscoveredField(field);
sb.addTerm(new Term(field, synonym));
}
posQuery = sb.build();
} else {
addDiscoveredField(field);
List<BooleanClause> innerClauses = new ArrayList<>();
for (String token : plist) {
onDiscoveredField(field);
innerClauses.add(new BooleanClause(super.getPrefixQuery(field, token), BooleanClause.Occur.SHOULD));
}
posQuery = getBooleanQuery(innerClauses);
Expand All @@ -707,7 +706,7 @@ private Query existsQuery(String fieldName) {
return new MatchNoDocsQuery("No mappings yet");
}

onDiscoveredField(fieldName);
addDiscoveredField(fieldName);

if (fieldNamesFieldType.isEnabled() == false) {
// The field_names_field is disabled so we switch to a wildcard query that matches all terms
Expand All @@ -734,7 +733,7 @@ protected Query getWildcardQuery(String field, String termStr) throws ParseExcep
}
List<Query> queries = new ArrayList<>();
for (Map.Entry<String, Float> entry : fields.entrySet()) {
onDiscoveredField(entry.getKey());
addDiscoveredField(entry.getKey());
Query q = getWildcardQuerySingle(entry.getKey(), termStr);
assert q != null;
queries.add(applyBoost(q, entry.getValue()));
Expand All @@ -760,7 +759,7 @@ private Query getWildcardQuerySingle(String field, String termStr) throws ParseE
}
if (forceAnalyzer != null && (analyzeWildcard || currentFieldType.getTextSearchInfo().isTokenized())) {
setAnalyzer(forceAnalyzer);
onDiscoveredField(currentFieldType.name());
addDiscoveredField(currentFieldType.name());
return super.getWildcardQuery(currentFieldType.name(), termStr);
}
if (getAllowLeadingWildcard() == false && (termStr.startsWith("*") || termStr.startsWith("?"))) {
Expand Down Expand Up @@ -808,7 +807,6 @@ protected Query getRegexpQuery(String field, String termStr) throws ParseExcepti
for (Map.Entry<String, Float> entry : fields.entrySet()) {
Query q = getRegexpQuerySingle(entry.getKey(), termStr);
assert q != null;
onDiscoveredField(entry.getKey());
queries.add(applyBoost(q, entry.getValue()));
}
if (queries.size() == 1) {
Expand All @@ -827,7 +825,7 @@ private Query getRegexpQuerySingle(String field, String termStr) throws ParseExc
return newUnmappedFieldQuery(field);
}
setAnalyzer(getSearchAnalyzer(currentFieldType));
onDiscoveredField(field);
addDiscoveredField(field);
return super.getRegexpQuery(field, termStr);
} catch (RuntimeException e) {
if (lenient) {
Expand All @@ -849,7 +847,7 @@ protected Query getBooleanQuery(List<BooleanClause> clauses) throws ParseExcepti
}

public Set<String> getDiscoveredQueryFields() {
return this.discoveredQueryFields;
return Collections.unmodifiableSet(this.discoveredQueryFields);
}

private Query applySlop(Query q, int slop) {
Expand Down