Skip to content

Fixed quote_field_suffix in query_string #29332

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

Merged
merged 4 commits into from
Apr 4, 2018
Merged

Fixed quote_field_suffix in query_string #29332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8008037
Fixed quote_field_suffix in query_string
jimczi Apr 2, 2018
0abf44c
address review
jimczi Apr 4, 2018
735a468
Merge branch 'master' into query_string_quote_field_suffix
jimczi Apr 4, 2018
a235775
Merge branch 'master' into query_string_quote_field_suffix
jimczi Apr 4, 2018
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
3 changes: 3 additions & 0 deletions server/src/main/java/org/elasticsearch/index/search/QueryStringQueryParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import static org.elasticsearch.common.lucene.search.Queries.newLenientFieldQuery;
import static org.elasticsearch.common.lucene.search.Queries.newUnmappedFieldQuery;
import static org.elasticsearch.index.search.QueryParserHelper.resolveMappingField;
import static org.elasticsearch.index.search.QueryParserHelper.resolveMappingFields;

/**
* A {@link XQueryParser} that uses the {@link MapperService} in order to build smarter
Expand Down Expand Up @@ -264,6 +265,8 @@ private Map<String, Float> extractMultiFields(String field, boolean quoted) {
// Filters unsupported fields if a pattern is requested
// Filters metadata fields if all fields are requested
return resolveMappingField(context, field, 1.0f, !allFields, !multiFields, quoted ? quoteFieldSuffix : null);
} else if (quoted && quoteFieldSuffix != null) {
Copy link
Member

Choose a reason for hiding this comment

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

When looking at how the field suffix is handled in SimpleQueryStringQuery#newPhraseQuery I saw there is a helper called QueryParserHelper.resolveMappingFields which might be reused here. At least the new unit tests pass when using that helper. It does a little more than the code here, so I'm not sure if that would be a good idea though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I pushed 0abf44c

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, thats what I tried locally. I wasn't sure it works in this case though because it also seems to resolve some regex patterns around wildcards etc... and I thought maybe this isn't wanted here, but glad it can be reused.

return resolveMappingFields(context, fieldsAndWeights, quoteFieldSuffix);
} else {
return fieldsAndWeights;
}
Expand Down
31 changes: 31 additions & 0 deletions server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,37 @@ public void testQuoteAnalyzer() throws Exception {
assertEquals(expectedQuery, query);
}

public void testQuoteFieldSuffix() throws IOException {
assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
QueryShardContext context = createShardContext();
assertEquals(new TermQuery(new Term(STRING_FIELD_NAME, "bar")),
new QueryStringQueryBuilder("bar")
.quoteFieldSuffix("_2")
.field(STRING_FIELD_NAME)
.doToQuery(context)
);
assertEquals(new TermQuery(new Term(STRING_FIELD_NAME_2, "bar")),
new QueryStringQueryBuilder("\"bar\"")
.quoteFieldSuffix("_2")
.field(STRING_FIELD_NAME)
.doToQuery(context)
);

// Now check what happens if the quote field does not exist
assertEquals(new TermQuery(new Term(STRING_FIELD_NAME, "bar")),
new QueryStringQueryBuilder("bar")
.quoteFieldSuffix(".quote")
.field(STRING_FIELD_NAME)
.doToQuery(context)
);
assertEquals(new TermQuery(new Term(STRING_FIELD_NAME, "bar")),
new QueryStringQueryBuilder("\"bar\"")
.quoteFieldSuffix(".quote")
.field(STRING_FIELD_NAME)
.doToQuery(context)
);
}

public void testToFuzzyQuery() throws Exception {
assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);

Expand Down